fix: update modulePreload polyfill, dedupe same href fetch

[patak-dev]

Description

Apply guybedford/es-module-shims#164. I simplified the cache to use a boolean instead of keeping the promise. I think it is better to avoid that.

We thought with @benmccann that this be related to #5532, but I tested it and the double request is there. The polyfill is simple in that regard, it is doing a direct fetch, and then the import will ask for the resource again. I was always assuming that the browser is able to work in this scenario as @sapphi-red commented on the issue.

I think we could merge this one just to be closer to the current state of es-module-shims.

This may also be related:

• Verify modulepreload behaviour in Firefox / Safari guybedford/es-module-shims#354

But it is strange because we had the same behavior for the past two years. If we can't find a way around this, we should look into disabling modulepreload when it isn't supported instead of polyfilling it in the next minor or major version.

---

What is the purpose of this pull request?

• Bug fix
• New Feature
• Documentation update
• Other

[sapphi-red]

IIUC this part only makes sense with es-module-shims.
es-module-shims uses fetch to fetch the module and executes them with import(blobUrl).
https://github.com/guybedford/es-module-shims/blob/e8eb942fdd65de8669184a4c1d8ad7dde21ea576/src/es-module-shims.js#L297
https://github.com/guybedford/es-module-shims/blob/e8eb942fdd65de8669184a4c1d8ad7dde21ea576/src/es-module-shims.js#L233

Because we don't fetch modules on our side and rely on the browsers mechanism, fetchCache won't be used when the modules are fetched.
So this fetchCache[link.href] part does the same thing with link.ep.

[patak-dev]

Oh, I missed this one. I didn't even look for the cache as I assumed it was local because I thought module preload polyfill should work with regular imports. Is your understanding of the polyfill that we can't extract the module preload and we should use the whole thing? The polyfill isn't small and IMO we shouldn't mess with the way modules are imported to support module preload. Not by default, and users could add this polyfill themselves if they want that.

But I think fetchCache was added to fix an edge case, and it was working in the general case without it (even if there is a double request).

So, we should close this PR. Then find out if the polyfill is really working. I need some help evaluating this, I thought it was working but I'm not sure anymore now.

Maybe for 4.2 we should get back to the original use of 'preload' in browsers that don't support 'modulepreload'. See cb75dbd#diff-121079017d1fa98d6a0ea3354d5c73df45ab277078228cd7384c7a4e84c5a813L35.

[bluwy]

Yeah I agree too. Looks like fetchCache doesn't have a benefit here. I think reverting to using preload would be the best bet for now too if modulepreload doesn't work.

[sapphi-red]

Is your understanding of the polyfill that we can't extract the module preload and we should use the whole thing?

I'm not sure about this. But at least this fetchCache part cannot be extracted. If this fetchCache mechanism is necessary to make the polyfill work, we can't extract only this part.

The polyfill isn't small and IMO we shouldn't mess with the way modules are imported to support module preload.

I agree to this.

Then find out if the polyfill is really working.

When I added this comment (#5532 (comment)), firefox did work when Cache-Control was set. I don't know whether it works now though.

[Rich-Harris]

I've been playing around with this stuff a fair bit over the last few days, and I've come to believe that modulepreload simply can't be polyfilled. It seems plausible that browsers have changed their behaviour since the polyfill was written, such that fetch and import/modulepreload just put things in different buckets. I would love to be proven wrong!

There is hope though: @jridgewell taught me this week that in Chrome, this...

<link rel="preload" as="script" crossorigin="anonymous" href="./something.mjs">

...works equivalently to modulepreload in Chrome (i.e. no double requests, the code is parsed as a module not a script, and import chains are followed) while also working in Safari (albeit without following import chains, which shouldn't matter here since the graph is flattened). Firefox requests files twice, unfortunately, but 🤷.

There is a catch though: the href must have a .mjs extension, not .js. This is what tells Chrome to treat it the same as a modulepreload.

[patak-dev]

We can't apply guybedford/es-module-shims#164 without using the whole polyfill. This fix doesn't seem related to #5532 at the end, as @sapphi-red and
guybedford/es-module-shims#354 (comment) explains, the request would show duplicated in Firefox but the second one should be a cache hit (only in prod?).

I'm going to close this PR. Right now, I'm not sure we should change the modulepreload default strategy. Each project can avoid the polyfill using modulePreload.polyfill: false. If someone else thinks we should move forward with changing this default in 4.2 let us know, we could review it again in 5.0 if not.

[benmccann]

Just FYI, the way we ended up handling this in SvelteKit was to create a preloadStrategy config with three options: https://kit.svelte.dev/docs/configuration#output