-
-
Notifications
You must be signed in to change notification settings - Fork 431
Preloading assets in Rollup apps #408
Comments
I'd like to suggest we also inject |
Any chance we could get one of the PRs merged in with the fix? I would love to increase my Lighthouse score 😃 |
Static sites can benefit from link headers as well, depending on how they are deployed. For instance with Netlify you can create a _headers file which specifies the headers for each path: https://www.netlify.com/docs/headers-and-basic-auth/ Would be great if we could export the headers somehow, or if there is a place to hook in a plug-in to create the _headers file. |
The headers should be preferred over the |
I sent a PR that implements this for JS files when using Rollup: #1269 We could probably implement it for CSS files and maybe webfonts or other files, but it's a pretty good start and probably the most meaningful piece to get done. |
@Rich-Harris is Sapper supposed to do code splitting on CSS files? It seems like rollup-plugin-svelte may be broken with Rollup 2 sveltejs/sapper-template#233. I can't make further progress on this without some advice from you or maybe @Conduitry or @antony on how the CSS is intended to work and probably someone with more Rollup experience fixing that issue assuming it is indeed an issue |
@benmccann interesting, I'd not noticed a difference. Ill check as soon as I'm able. |
Not sure if relevant but might this be something to take a look at: https://github.com/vikerman/rollup-plugin-hoist-import-deps ? I'm using this on https://svelte.school with good results :) |
@kevmodrome thanks for the pointer. I'd actually taken a look at that plugin. It's helpful, but #1269 does even better. You can see my findings here: #1260 (comment). Anyways, that plugin and the work I've done so far just address the JS piece. My remaining question is about the CSS. I'm happy to put together a PR address the CSS piece as well, but think we should figure out the Rollup 2 compatibility first |
Just as an addition to issue sveltejs/sapper-template#233 pointed by @benmccann, I had actually posted another issue in rollup/rollup#3655 along with the repro details. |
With much help from the Rollup team, I was able to fix CSS code splitting in Sapper for Rollup 2 and sent a PR for it: #1306. For Rollup 2 users will need to use Rollup 2.21.0+ and will need #1306 merged in order to get code splitting. Otherwise everything is loaded as one big CSS file With that fixed, I've gone ahead and updated my PR #1269 to |
Hello, got any update when this merge will happen? Thx |
Hello, are there any needed configurations in Rollup or Sapper to benefit from this? |
No configuration is necessary |
Any package update? |
Yes, you need to use Sapper 0.28.0 |
ok, thank you |
got this message: |
That's unrelated to this PR. Please see the migration guide: https://sapper.svelte.dev/migrating#0_27_to_0_28 |
Did all the steps and still have the message, also on the realworld project after sapper update getting the same message |
It's the last bullet in the migration guide. It works fine on the realworld project. See my PR that's pending there to do the upgrade If you have further issues, please ask in Discord. The warning you're seeing is unrelated to the feature discussed in this ticket |
All is fine, thank you. |
Currently, only
client.xyz123.js
is preloaded. Sapper should add preload headers for all the JS and CSS needed for the page being server-renderedThe text was updated successfully, but these errors were encountered: