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

Verify modulepreload behaviour in Firefox / Safari #354

Closed
guybedford opened this issue Feb 15, 2023 · 7 comments
Closed

Verify modulepreload behaviour in Firefox / Safari #354

guybedford opened this issue Feb 15, 2023 · 7 comments

Comments

@guybedford
Copy link
Owner

Apparently browsers may have changed their behaviour, it would be good to verify this is still working correctly or needs adjustment.

@Rich-Harris
Copy link
Contributor

Subscribing, as I would love to be proven wrong!

@guybedford
Copy link
Owner Author

guybedford commented Feb 23, 2023

I did some tests against Firefox today (still have to check Safari), and found the following:

  1. Server sending ETag: fetch and <link rel=preload as=script> both send a new request for the module, but at least the if-none-match header is sent for the preloaded content, so that a 304 happens on the second request. In theory this shouldn't add major additional network overhead, and this is probably still the most recommended polyfill approach.
  2. Server not sending Etag, but just expires - cache is used.

I still need to do the Safari tests, but based on the above at least my plan for now is to update es-module-shims with a new configuration modulepreloadShim: false to disable this in cases where it isn't wanted. That said, most applications should really be using etags, so I hope it can remain a best-practise and enabled by default.

Once I've finished the above I'll update the blog post as well.

I believe the change that happened here was Firefox supporting Sec-Fetch-Dest when no ability was given to the fetch() API to support setting the destination (just various issues in which it was discussed but never followed-up). I'm not sure exactly which Firefox version this change happened in though, but there was definitely a change here.

@guybedford
Copy link
Owner Author

guybedford commented Feb 23, 2023

Unfortunately by the nature of browser module loading, the flat preloads in the non-expires case will just result in a "304 waterfall". In theory a 304 waterfall should be quicker than a full content waterfall, but it's still a waterfall which somewhat defeats the point sadly.

At least both Firefox and Safari have now landed modulepreload since this was opened!

@guybedford
Copy link
Owner Author

In Safari, we have a better story thankfully - and there is no duplicate request at all with full cache sharing. In Safari <link rel=preload> shares the full request pipeline, while fetch uses the disk cache only.

The waterfall is thus entirely avoided in all webkit engines.

In theory the choice between fetch() and <link rel=preload> could be seen as an optimization versus correctness problem now. Perhaps it's time to make this configurable too.

@guybedford
Copy link
Owner Author

Update on the expires header in Firefox - I was misreading the tests, and expires are fully respected. It's only the case of sending an ETag without an Expires that results in the 304 check only as a worst case. All other caching scenarios work out, although they are still network cache usages through the entire network pipeline (so involving process + memory copy costs, not network costs though).

@guybedford
Copy link
Owner Author

Marking as completed, as I've verified these cases are working well for the most part and as expected.

Any issue here can probably be attributed to #359 instead.

@guybedford
Copy link
Owner Author

@Rich-Harris if you have any counter examples to the above please do let me know though, but it seems to be working to me. Yes the entry will appear twice in the network panel, but the second time it should at least appear as cached.

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

No branches or pull requests

2 participants