Skip to content
This repository was archived by the owner on Jan 11, 2023. It is now read-only.

Preloading assets in Rollup apps #408

Closed
Rich-Harris opened this issue Sep 2, 2018 · 22 comments · Fixed by #1269
Closed

Preloading assets in Rollup apps #408

Rich-Harris opened this issue Sep 2, 2018 · 22 comments · Fixed by #1269

Comments

@Rich-Harris
Copy link
Member

Currently, only client.xyz123.js is preloaded. Sapper should add preload headers for all the JS and CSS needed for the page being server-rendered

@maxmilton
Copy link

I'd like to suggest we also inject <link href=xxx rel=preload as=xxx> and <link href=xxx rel=prefetch as=xxx> into the document head for sapper export so static builds can also benefit. The headers are great since some systems can use them for HTTP/2 push but the reality is it's still early days for push.

@RobbieTheWagner
Copy link

Any chance we could get one of the PRs merged in with the fix? I would love to increase my Lighthouse score 😃

@andrewagain
Copy link

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.

@benmccann
Copy link
Member

benmccann commented Jun 3, 2020

The headers should be preferred over the link tags because the preload spec mentions that servers may initiate a PUSH when they see a preload header and most CDNs have standardized around this behavior to initiate a PUSH.

This was referenced Jun 10, 2020
@benmccann
Copy link
Member

benmccann commented Jun 13, 2020

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.

@benmccann
Copy link
Member

@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

@antony
Copy link
Member

antony commented Jul 4, 2020

@benmccann interesting, I'd not noticed a difference. Ill check as soon as I'm able.

@kevmodrome
Copy link
Contributor

kevmodrome commented Jul 4, 2020

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 :)

@benmccann
Copy link
Member

@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

@habibrosyad
Copy link
Contributor

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.

@benmccann
Copy link
Member

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 preload CSS which was fairly straight-forward since most of the work was already done and we just needed to connect some pieces. It will work with any version of Rollup 1 and Rollup 2, but you will need the fix I just mentioned and latest version of Rollup to get code splitting if using Rollup 2

@LucianVoju
Copy link

Hello, got any update when this merge will happen? Thx

@LucianVoju
Copy link

Hello, are there any needed configurations in Rollup or Sapper to benefit from this?

@benmccann
Copy link
Member

No configuration is necessary

@LucianVoju
Copy link

Any package update?

@benmccann
Copy link
Member

Yes, you need to use Sapper 0.28.0

@LucianVoju
Copy link

ok, thank you

@LucianVoju
Copy link

got this message:
'preload' is not exported by 'src/routes/_layout.svelte'
333:
334: if (!root_preloaded) {
335: const root_preload = root_comp.preload || (() => {});
^
336: root_preloaded = initial_data.preloaded[0] || root_preload.call(preload_context, {
337: host: page.host,

@benmccann
Copy link
Member

That's unrelated to this PR. Please see the migration guide: https://sapper.svelte.dev/migrating#0_27_to_0_28

@LucianVoju
Copy link

Did all the steps and still have the message, also on the realworld project after sapper update getting the same message

@benmccann
Copy link
Member

benmccann commented Aug 9, 2020

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

@LucianVoju
Copy link

All is fine, thank you.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
10 participants