kit icon indicating copy to clipboard operation
kit copied to clipboard

Support <link rel="modulepreload"> for SSR

Open thoughtspile opened this issue 2 years ago • 4 comments

Describe the problem

Following https://github.com/sveltejs/kit/pull/5735 by @benmccann svelte-kit only provides list of scripts to preload in the link header. This has several issues:

  • Long link headers cause hard-to-debug reverse proxy (e.g. nginx) errors, see https://github.com/sveltejs/kit/issues/6790 This is very dangerous as proxy settings may vary between dev / staging / production and the app might unexpectedly explode in production. Moreover, docs and changelog don't detail this behavior.
  • From what I can tell, link header is experimental and only supported in chrome@103+, while <link> tags work in chrome@66

Describe the proposed solution

Since the code to inject <link rel="modulepreload"> exists for prerendering case anyways (see https://github.com/sveltejs/kit/blob/5e12d3e42b981c857455904a169b5733fa514e09/packages/kit/src/runtime/server/page/render.js#L300), the old behavior with <link> tags can be enabled via svelte.config.js option, such as modulepreload: 'tag' | 'header' with little added complexity.

Alternatives considered

@Rich-Harris suggests removing link header altogether (see https://github.com/sveltejs/kit/discussions/6519#discussioncomment-3703967 and https://github.com/sveltejs/kit/issues/6790#issuecomment-1254205734) to fix the issue. My understanding is that it delays preloading altogether, falling back to preload-helper run in start.js, which is not optimal.

It's also possible to manually convert link header to a list of <link> tags in a middleware, e.g.

response.headers.delete('link');
const link = response.headers.get('link');
const body = await response.text();
return new Response(body.replace('</head>', `${linkHeaderToHtml(link)}</head>`), response);

However, this can't be done in a normal transformPageChunk since link header is not available before await resolve, and involves fully buffering response body, preventing streaming response, which is, again, problematic.

Importance

would make my life easier

Additional Information

No response

thoughtspile avatar Jan 16 '23 10:01 thoughtspile

modulepreload isn't supported either by Safari/Firefox, so we wouldn't gain anything from adding a config option here in terms of browser compatibility.

I'm wondering if we should print a warning or something similar when the header becomes too long so that it's easier to discover.

dummdidumm avatar Jan 16 '23 12:01 dummdidumm

Vite polyfills modulepreload so it actually would work in other browsers (we might have disabled it, but we could change that or make it dependent on the proposed option)

I'm not sure how we'd print a warning because we're not sure what too long is as it depends on the user's setup

The reason we thought the header approach was interesting is that we could send the headers before generating the body which would provide more of a performance boost. But the length issue and not working in other browsers are both big caveats, so I'm not sure it's the best approach. I'd probably support an option and changing the default.

benmccann avatar Jan 16 '23 14:01 benmccann

we wouldn't gain anything from adding a config option here in terms of browser compatibility.

As far as browser support goes, it seems like chrome between 66 and 103 works with <link> tag, but not link header. With safari / FF it's unclear if <link rel="modulepreload" support is added earlier than link header support, so it's speculative.

Vite polyfills modulepreload

Correct, but the polyfill only works after start.js is loaded and executed (please correct me if I'm wrong), while native link starts downloading assets right after HTML arrives. Granted, I have no real-world timing data to say if it has a sizable effect.

The header approach is certainly very interesting, but I don't think it should be enabled by default given the length issue — especially since, as you correctly noted, we can't generate a warning. For now it appears that <link> is a safer one-size-fits-all alternative.

thoughtspile avatar Jan 16 '23 14:01 thoughtspile

On a related note — if svelte-kit supports HTML response streaming, the response fragment with links can probably be flushed before rendering the main content. This preload will be invalidated if page handler redirects or throws, but same applies to header approach. Not sure if svelte-kit supports, or plans to support, HTML streaming?

To add more speculation, HTML links can be compressed (they contain a lot of repetition), while I'm not sure if header compression is applied in common setups.

thoughtspile avatar Jan 16 '23 14:01 thoughtspile

Not sure if svelte-kit supports, or plans to support, HTML streaming?

Not currently. We've talked about flushing <head> content early (excluding <svelte:head> content) and streaming the body later, but we'd need a strategy for handling the case where an error occurs during data loading or rendering

Rich-Harris avatar Jan 28 '23 00:01 Rich-Harris

@Rich-Harris you removed the prerendering check in the "separate app/framework" PR, do you want to reconsider the headers or can this be closed?

gtm-nayan avatar Feb 21 '23 08:02 gtm-nayan

This hasn't been mentioned before: the output of the "link" response headers can be controlled by the option preload in a server hook:

export const handle: Handle = async ({ event, resolve }) => {
    return await resolve(event, {
        preload: ({ type, path }) => type === 'font', // suppress js and css link headers
    });
};

See https://kit.svelte.dev/docs/hooks#server-hooks for details.

dmoebius avatar Sep 09 '23 00:09 dmoebius