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

Replace modulepreload with preload for Firefox and Safari #1387

Open
benmccann opened this issue Aug 11, 2020 · 14 comments
Open

Replace modulepreload with preload for Firefox and Safari #1387

benmccann opened this issue Aug 11, 2020 · 14 comments

Comments

@benmccann
Copy link
Member

modulepreload is only supported by 71% of browsers: https://caniuse.com/#search=modulepreload
preload is supported by 87% (soon to be > 90% since Firefox beta has support): https://caniuse.com/#search=preload

There's no way to make modulepreload work since our JS files have dependencies on each other: https://developers.google.com/web/updates/2017/12/modulepreload

I'm not aware of a compelling reason to build the /client/ as ES6 modules

@ajbouh
Copy link

ajbouh commented Aug 11, 2020

Based on my understanding we should be able to flatten out the dependency graph and include all of them in a set of individual modulepreload directives.

The implementation I have locally will emit preload module preload directives based on a boolean. Once I have everything wired up and in production I should be able to provide some real world performance info about what sort of difference different configurations make.

@benmccann
Copy link
Member Author

Based on my understanding we should be able to flatten out the dependency graph and include all of them in a set of individual modulepreload directives.

We do that today, but caniuse says it's only supported in Chrome

The line from the article above that I'm concerned about is:

This means that you would have to change the crossorigin attribute in both your <script> and to one of the other values, and you might not have an easy way of doing so if what you're trying to preload is a dependency of other modules.

So eventhough we can preload the script easily enough, the browser will consider it to be a different script when it encounters it as a dependency. (It's a really helpful article, so I recommend reading the whole thing - especially the lines around the one I quoted)

@ajbouh
Copy link

ajbouh commented Aug 11, 2020

Yeah, I read the article ... Agree it's a good one. I my thinking is that the caveat doesn't apply to sapper though, since the assets are not served in a crossorigin way, and none of the other scripts on the page that would reference the sapper modules.

Does this seem incorrect to you?

@benmccann
Copy link
Member Author

benmccann commented Aug 11, 2020

With the way that Rollup does code splitting you'll end up with some of the Sapper scripts referencing other scripts. My understanding is that dependencies will be loaded with credentials mode of omit, which doesn't exist for <link rel="preload">

@ajbouh
Copy link

ajbouh commented Aug 11, 2020

Except doesn't credentials omit only come into play if you're loading modules from other origins?

@benmccann
Copy link
Member Author

From Medium:

Unlike regular scripts, module scripts (and their imports) are fetched with CORS.

@ajbouh
Copy link

ajbouh commented Aug 11, 2020

Ok, after reading that and a bit more spelunking through the spec and corresponding browser issue trackers, it seems like preload-based performance optimizations are not something everyone can take for granted yet.

It does seem like parsing of code fetched via preloadmodule is expected to be faster than preload alone, since more of the processing can be handled off the main thread.

All things considered, it might make sense to base the decision to move off of esm on real world performance measurements if we can get them.

@benmccann
Copy link
Member Author

Ah, yeah, you're right that preloadmodule should be faster than preload. But I'm not sure how much faster and wonder if it'd be enough we'd want to build and serve a separate JS for Chrome than Safari and Firefox. I was kind of assuming we'd rather avoid the complication. But I guess the best would be to make it easier for the user to have control over stuff like this

@ajbouh
Copy link

ajbouh commented Aug 12, 2020

While reading https://docs.google.com/document/d/1WebH4IOCswACUbaczx5cGQPVl5mnqcieOd4MRJM2syk/edit I saw

Now module scripts are fetched in "same-origin" credentials mode, so can be used to match the credentials mode.

Which links to the PR back in 2018 that "fixed" the preload spec for modules: whatwg/html#3656

A comment on that PR suggests that Firefox follows the spec (I think @benmccann found that it's still not on by default):

https://bugzilla.mozilla.org/show_bug.cgi?id=1493449 (default credentials handling for module fetches) is fixed for Firefox 64.

@benmccann
Copy link
Member Author

Wow, good find! I guess the Google post that comes up in search results is outdated. I just looked at one of the files that Sapper's loading and it has a sec-fetch-site: same-origin header.

So it seems that the rel="preload" fallback for non-Chrome browsers probably will work though we may need to add crossorigin in that case from what I'm seeing reported on various posts around the web

@benmccann benmccann changed the title Don't build ES6 modules for client and replace modulepreload with preload Replace modulepreload with preload for Firefox and Safari Aug 12, 2020
@ajbouh
Copy link

ajbouh commented Aug 13, 2020

Is there a precedent for doing browser/user-agent-specific things in SSR yet?

@benmccann
Copy link
Member Author

No, there isn't. I'd try to avoid it as much as possible. But in this case we either have to do that or replace modulepreload with preload for everyone

@ajbouh
Copy link

ajbouh commented Aug 13, 2020

The fact that chrome wants to stop sending the User-Agent header doesn't help the situation either :/

Hopefully I'll start having some real world performance data in the near future.

@benmccann
Copy link
Member Author

You'll still be able to tell they're running Chrome and desktop vs mobile though. Just not much else like what OS

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

No branches or pull requests

2 participants