Double and triple loading with modulepreload #148

dhh · 2021-08-16T07:59:19Z

Given this page:

<!DOCTYPE html>
<html>
<head>
  <script async src="https://cdn.jsdelivr.net/npm/[email protected]/dist/es-module-shims.min.js"></script>
  <link rel="modulepreload" href="https://cdn.jsdelivr.net/npm/[email protected]/dist/vue.esm.browser.js">
  <script type="importmap">
    { "imports": { "vue": "https://cdn.jsdelivr.net/npm/[email protected]/dist/vue.esm.browser.js" } }
  </script>
  <script type="module">
    import Vue from 'vue'
    new Vue({ el: '#app-4', data: { todos: [ { text: 'Learn JavaScript' }, { text: 'Learn Vue' }, { text: 'Build something awesome' } ] } })
  </script>
</head>
<body>
<div id="app-4">
  <ol>
    <li v-for="todo in todos">
      {{ todo.text }}
    </li>
  </ol>
</div>
</body>
</html>

I see Vue loaded twice in Chrome 92, both from network:

I see Vue loaded thrice in Safari 14.1.1, but only the first from network:

I see Vue loaded thrice in Firefox 88, twice from network:

If I remove es-module-shim, it's only loaded once, correctly, in Chrome 92.

If I remove the modulepreload, it's only loaded once in Chrome 92, but still twice from network in Safari and once from network, once from cache in Firefox.

I assume I'm doing something basic wrong, but not sure what?

guybedford · 2021-08-16T17:59:53Z

Thanks for the clear reports.

Chrome is somehow partitioning the cache to not share the fetch and module network caches. This seems new and did not used to be the case and eg is not the case in Brave or Edge.
Safari is correct - there are three requests - the preload, the module fetch, and the es module shims analysis fetch. These three requests should always share the network cache which it seems they are thankfully.
The Firefox "race" sounds like it i between the fetch for the preload polyfill and the fetch for the actual resource. This could be resolved by having the es module shims preload polyfill maintaining an internal fetch cache that it shares with its internal fetch as a way to work around the Firefox double network request. That sounds like a viable workaround and I'll aim to get a PR up soon for this.

For the Chrome issue I am following up with browser vendors. I assume this is new as of Chrome ~91+, so it may just be possible to get a bug report and fix, although if not that certainly puts the viability of this project into question.

dhh · 2021-08-16T19:54:49Z

Hopefully @domenic could help motivate some interest on the Chrome team. For now, I’m just short circuiting all the feature testing with a crude return before anything runs if navigator.userAgent is Chrome 😄. But of course then Chrome complains that we should be feature testing instead 😂.

Nice to see Safari playing nice for once!

Also realized that Chrome has another bug with modulepreload and import maps. Basically, it works on first load but then fails on reload while maintaining the cache. That’s with the native support though. So maybe that regression is related to the issue seen here as well. Will try to reproduce with a small test case for them.

Really appreciate the work on this! Importmaps are super exciting. And this is what actually allows it to be a viable path for production apps 🙏🙏

guybedford · 2021-08-17T00:11:12Z

I've posted and landed #149 which resolves the Firefox issue here.

I had a brief discussion with @domenic and it didn't sound promising to get the cache sharing between modules and fetch with credentials cross-origin sharing a network cache entry / key.

As a result I have instead posted an alternative proposal to allow polyfill mode to avoid double fetches in modern Chrome in #150.

There is one other possibility I was considering and that is doing global error detection to catch the module resolution error message failures and then having polyfill mode run that way, but that feels like a very brittle and complex approach I'd rather steer away from for now. Hopefully adding a polyfill="importmap" attribute doesn't seem to onerous? Would value your feedback on that!

And thank you also for your adoption and education around these features, it is a huge boost to the ecosystem!

guybedford · 2021-08-18T01:58:25Z

I just tested out #149 against the original test case here and it seems that that actually has resolved the double network issue in Chrome as far as I can tell, which I hadn't expected.

I've just released 0.12.3 with this fix, please test it out and let me know how that seems there.

Then we can follow up further on #150 as necessary. It would still be a minor perf improvement for large applications to avoid unnecessary lexer work (~10ms compute per MB on startup say for the lexer, plus probably something like ~5-20ms per MB further processing work time I'd expect, would need tests to properly verify though).

domenic · 2021-08-18T22:50:58Z

Although I'm glad that this got resolved, I realized after inspecting the screenshots in the OP that I was misunderstanding the issue in our DM conversation, @guybedford. Two actual network requests is not what I expected; I expected the network cache to be used and the doubling up to be at a different layer.

If you could produce a minimal-ish example, or even a non-minimal one, of double network requests for the same resource, that would be helpful as a Chromium bug. (Contrary to what I was saying in our earlier conversation.) It might still be intended behavior---for example, it looks like the first request wasn't finished yet, so maybe Chrome thought it was stalling out and re-requested a second one instead of waiting?---but it might also be a bug.

My apologies for not understanding earlier.

guybedford · 2021-08-19T05:05:01Z

@domenic that's very relieving to hear that the network cache is expected to have these guarantees, and thanks for the update on that. I believe the reason for the issue was that the preload technique was doing just fetch(url) instead of fetch(url).then(res => res.ok && res.text()) to fully read through the response. As a result the network cache likely wasn't populated yet to be shared. I'd be interested to hear if that sounds like a possible explanation here to you, at least that seems like the most sensible explanation from testing the various approaches.

dhh · 2021-08-19T13:52:13Z

On a force reload in Chrome 92, I still see the double loading with 0.12.4:

guybedford · 2021-08-19T17:10:37Z

Ah that is a shame, yes I can replicate the issue is still there. I use Brave as my main browser assuming it behaves the same as Chrome in many cases but it turns out Brave has an entirely different processing model for cache keys.

@domenic here is a very simple replication of a double network request:

<!DOCTYPE html>
<html>
<head>
  <script type="module">
    import Vue from 'https://cdn.jsdelivr.net/npm/[email protected]/dist/vue.esm.browser.js';

    // results in double fetch
    fetch('https://cdn.jsdelivr.net/npm/[email protected]/dist/vue.esm.browser.js', { credentials: 'same-origin' })
    .then(res => res.ok && res.text());
  </script>
</head>
</html>

It can also be switched around with the fetch first, which more closely resembles the preload polyfill as well.

Would be interested to hear your thoughts again on whether it is worth posting a browser bug here.

@dhh it sounds like progressing work on #150 will be the best approach here for production optimization at least in the mean time. I will put some time aside to work on the PR further.

domenic · 2021-08-19T17:16:32Z

Yeah, I think that's worth posting a browser bug. I'm not confident on what the outcome would be---maybe it's even just a devtools artifact??---but it's worth investigating.

guybedford · 2021-08-19T17:28:14Z

Ok I've posted https://bugs.chromium.org/p/chromium/issues/detail?id=1241493. Edge and Firefox actually do have the same issue as well. It's because Firefox doesn't support import maps yet that it doesn't yet affect es-module-shims. Safari and Brave are the only ones that do successfully coalesce.

guybedford · 2021-08-19T18:13:09Z

Ok the follow up on the chromium bug is that the double network requests seem to come up for:

When "disable cache" in network panel is used
When doing a "hard refresh" or "clear cache and hard refresh".

On the other hand we can get the correct single request when doing the following:

Ctrl + Shift + Del -> Clear Data
Then doing a soft refresh on the page (Ctrl + R, NOT Ctrl + Shift + R)

With the above we've been able to ascertain that this does in fact correctly coalesce as a single request for the soft refresh (normal browsing) case.

@dhh perhaps see if the above also works for you here?

dhh · 2021-08-21T06:43:54Z

Yes, I now see that same behavior. I'd say it's not ideal for Chrome, since the non-shimmed behavior is the better one. Curious, is it not possible to simply leave Chrome to its own devices for the behavior where it doesn't need shimming? Or do you need this to make the other shims work?

Also, interestingly, in incognito mode, Safari essentially acts as Chrome with disable cache turned on, so it'll double load even on first load with a clean cache:

I see single-load behavior in normal Safari and Firefox now 👍

guybedford · 2021-08-21T17:47:43Z

Thanks for confirming you can verify the same, it seems this is likely the best case on it at this point. I will close out the Chromium bug here as well.

Curious, is it not possible to simply leave Chrome to its own devices for the behavior where it doesn't need shimming?

This is exactly what #150 is tracking - basically the problem is we don't know if a given graph will execute in modern chrome if we don't know what features the graph uses. Upfront attributes on the module script that declare what features that are needed for that given graph can allow an earlier analysis opt-out.

dhh · 2021-08-21T17:51:42Z

Ah yes doh. Forgot about that for a second 👌

guybedford · 2021-08-24T05:32:38Z

The bug here has been resolved, let's follow up on optimizing work done in the tracking issue #150.

guybedford mentioned this issue Aug 16, 2021

feat: preload polyfill fetch cache #149

Merged

guybedford closed this as completed Aug 24, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Double and triple loading with modulepreload #148

Double and triple loading with modulepreload #148

dhh commented Aug 16, 2021

guybedford commented Aug 16, 2021 •

edited

Loading

dhh commented Aug 16, 2021

guybedford commented Aug 17, 2021

guybedford commented Aug 18, 2021

domenic commented Aug 18, 2021

guybedford commented Aug 19, 2021

dhh commented Aug 19, 2021

guybedford commented Aug 19, 2021

domenic commented Aug 19, 2021

guybedford commented Aug 19, 2021

guybedford commented Aug 19, 2021

dhh commented Aug 21, 2021

guybedford commented Aug 21, 2021

dhh commented Aug 21, 2021

guybedford commented Aug 24, 2021

Double and triple loading with modulepreload #148

Double and triple loading with modulepreload #148

Comments

dhh commented Aug 16, 2021

guybedford commented Aug 16, 2021 • edited Loading

dhh commented Aug 16, 2021

guybedford commented Aug 17, 2021

guybedford commented Aug 18, 2021

domenic commented Aug 18, 2021

guybedford commented Aug 19, 2021

dhh commented Aug 19, 2021

guybedford commented Aug 19, 2021

domenic commented Aug 19, 2021

guybedford commented Aug 19, 2021

guybedford commented Aug 19, 2021

dhh commented Aug 21, 2021

guybedford commented Aug 21, 2021

dhh commented Aug 21, 2021

guybedford commented Aug 24, 2021

guybedford commented Aug 16, 2021 •

edited

Loading