Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support <link rel="modulepreload"> for SSR #8549

Open
thoughtspile opened this issue Jan 16, 2023 · 7 comments
Open

Support <link rel="modulepreload"> for SSR #8549

thoughtspile opened this issue Jan 16, 2023 · 7 comments
Labels
feature request New feature or request
Milestone

Comments

@thoughtspile
Copy link

thoughtspile commented Jan 16, 2023

Describe the problem

Following #5735 by @benmccann svelte-kit only provides list of scripts to preload in the link header. This has several issues:

Describe the proposed solution

Since the code to inject <link rel="modulepreload"> exists for prerendering case anyways (see

head += `\n\t\t<link rel="modulepreload" href="${path}">`;
), 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 #6519 (comment) and #6790 (comment)) 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

@dummdidumm
Copy link
Member

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.

@benmccann
Copy link
Member

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.

@thoughtspile
Copy link
Author

thoughtspile commented Jan 16, 2023

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
Copy link
Author

thoughtspile commented Jan 16, 2023

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.

@Rich-Harris Rich-Harris modified the milestones: 2.0, soon Jan 28, 2023
@Rich-Harris Rich-Harris added the feature request New feature or request label Jan 28, 2023
@Rich-Harris
Copy link
Member

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

@gtm-nayan
Copy link
Contributor

@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?

@dmoebius
Copy link

dmoebius commented Sep 9, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request
Projects
None yet
Development

No branches or pull requests

6 participants