Sapper: [Rollup] Remove `client.js` from Link header

[lukeed]

On the Rollup version _only_, the bundle_info.assets.main JS bundle should not be included in the Link response header.

While that would technically be the correct action to take (and continues to be with the Webpack template), it results in a double-download of the main file on _any_ application produced with Rollup.

The reason is because the main JS file is passed thru Shimport in every scenario, and so the document cannot infer that the preloaded asset can be reused. This can be verified by removing the Link header and instead using link rel=preload as=script tags instead. In this case, there's a JS console warning about the client.*.js not being used within a few seconds, despite the fact that _was_... it was just that a second request was made instead.

Contrast this with the Webpack template, which _does_ have a script src=* tag directly on the page. Here, the browser can correctly infer that the preloaded asset goes <here>.

The easiest solution is to remove the client.*.js file from the preloaded_chunks array, and do a "raw" request or the file thru Shimport. Since the page is SSR'd & isn't interactive until all chunks have finished loading anyway (thru Shimport), this wouldn't delay the page's timeline by much.

^ I have a local fix for this & can push it up if desired.

The other option is to do the browser-compat checks _in the server middleware_ instead of waiting to begin on DOMContentLoaded. You'd sniff the UA string that comes in & serve the appropriate bundle-set immediately.

This is something I've seen @kristoferbaxter do well with his preact-hn and is something I've considered working into sirv – or perhaps another module that builds from it.

And finally, here's a network timeline for the base Rollup template, running on a simulated 3G.

[arxpoetica]

@lukeed what's your local fix? mind sharing?

[mrkishi]

This seems to happen regardless of whether shimport is being used or not.

I don't know what the best course of action is, but the browser (Chrome, at least) won't match the preloaded scripts with scripts of type="module". When using shimport, it won't match them because they should be fetch instead of script.

More info in this article, with an even more experimental feature modulepreload. I guess it would be safe to swap the current preload for a modulepreload, since the latter will only work on browsers that also do not load shimport.

Update: Actually, as of #459, adding a crossorigin="use-credentials" to the link header will cause a match. Could someone test it with Safari? The other browsers don't support preload yet (except for Edge, but not as a header), so if it also matches, simply adding crossorigin should be enough _for now_.

[lukeed]

@mrkishi Very interesting, thanks! I didn't know about this 🙌

@arxpoetica Sure, I didn't PR because it's not really a good solution IMO, but it _does_ work in that it prevents the double download.

In the latest version of Sapper, these lines should become:

        // TODO detect other stuff we can preload? images, CSS, fonts?
        let main_chunk = build_info.bundler === 'rollup' ? [] : build_info.assets.main;
        let preloaded_chunks = Array.isArray(main_chunk) ? main_chunk : [main_chunk];
        if (!error) {
            page.parts.forEach(part => {
                if (!part)
                    return;
                // using concat because it could be a string or an array. thanks webpack!
                preloaded_chunks = preloaded_chunks.concat(build_info.assets[part.name]);
            });
        }
        const link = preloaded_chunks
            .filter(file => file && !file.match(/\.map$/))
            .map(file => `<${req.baseUrl}/client/${file}>;rel="preload";as="script"`)
            .join(', ');
        if (link) res.setHeader('Link', link);

[lukeed]

Both of @mrkishi's suggestions seem to work as well, including on Safari

[arxpoetica]

Anyone want to do a PR of @mrkishi's fix? I'm not following at all :(

(I'm also worried this is the thing that's breaking my build in production, though I don't have evidence...)

[benmccann]

@lukeed is this still an issue? I sent https://github.com/sveltejs/sapper/pull/1269 which preloads all assets including the client.js and I'm not seeing client.js requested twice in the network tab

[lukeed]

Not sure tbh; nearly two years ago 🙈