-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
feat: add option for css preloads when prerendering #12247
Conversation
🦋 Changeset detectedLatest commit: 6d9a52f The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Looks good to me. @benmccann was there a reason we didn't use link tags to preload stylesheets even before we added Link headers in #5735 ? |
I think that it's probably because that in the general case it doesn't actually do anything and just adds extra page weight since CSS is already loaded with highest priority. This is kind of a Cloudflare-specific functionality. I feel like the best solution here would probably be for Cloudflare to send early hints for all CSS rather than just preloaded CSS so that we don't have to duplicate all the CSS tags as preload tags and so that more of their customers benefit from that functionality. |
@benmccann That's fair, do you think it would be in-scope to have |
I can see that working. We'd need to get the asset paths then add them all as a Link header key value in _headers. Maybe this can be done by reading the prerendered html files in the cloudflare adapter? or is there a better way to get them like through the Vite manifest? We also have to keep in mind the 100 rule limit for the _headers file (similar to _routes file). |
To keep the rules under the 100 rule limit, perhaps we can do some rule merging? e.g. set the header on In terms of actually getting the header information out of |
For this PR: I think the above changes I've suggested are probably more resilient long-term but harder to work out the implementation details. In the short term, would there be support for adding in |
@dario-piotrowicz any chance you might be able to put in a request to have Cloudflare send early hints for all CSS rather than just preloaded CSS? (see above for context: #12247 (comment). happy to clarify on Discord if that's easier) |
Dario suggested an interesting answer that might be the way to go here. I tested and, at least in firefox and chrome where I tested, you don't need both a tag to preload it and a tag to load it. You can simply preload it and that is enough. That would break IE users, but that ship may have sailed. Perhaps we should user agent sniff and not do it there though? Might have to take a look at #6265 to see how badly broken we are in IE right now |
c21e56f
to
16ba0ff
Compare
e47fa69
to
d1080ff
Compare
@benmccann Sorry for the super delayed response. I updated this feature to be gated behind a config option that defaults to false, since I'm not super confident about changing the default behavior to only emit using preload tags with the standard stylesheet |
`KitConfig.prerender.generateCssPreloadTags` will configure whether preload tags for stylesheets are generated during prerendering.
LGTM. @benmccann what do you think? It has a config option defaulting to false at the moment so it shouldn't break anything. |
We've generally tried to avoid adding options to reduce user complexity. I think always adding the preload in place of the normal css loading would be preferable. The framework should take care of these decisions for users. By creating an option we just add decisions that users are responsible for making |
Okay, will close based on discussion above. |
Adding a
<link rel="preload">
tag to prerendering for stylesheets is helpful for static page rendering. This is especially useful when deploying to Cloudflare, which looks for preload tags with its Early Hints feature.Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
Tests
pnpm test
and lint the project withpnpm lint
andpnpm check
Changesets
pnpm changeset
and following the prompts. Changesets that add features should beminor
and those that fix bugs should bepatch
. Please prefix changeset messages withfeat:
,fix:
, orchore:
.Edits