Modulepreload error caching
Closed this issue · 9 comments
@lewisl9029 alerted me to the issue he found in https://bugs.chromium.org/p/chromium/issues/detail?id=1313317.
It seems that if there is a <link rel="modulepreload" href="/app.js" />, where /app.js uses bare specifiers, and the preload comes before the import map, then when caching is used it is possible for the preload to complete before the import map has been processed resulting in the bare specifier error being cached.
As a result, when one later imports <script type="module" src="/app.js">, the cached bare specifier error stops the module load from processing instead of returning the correct module.
Seems like it would be worth ironing this one out - questions:
- Is this the intended behaviour according to the preload, module and import maps specifications?
- If it is intended, then perhaps not caching errors will exactly alleviate this issue
- Alternatively, perhaps import maps need to specify that module preload should wait for import maps somehow either with a
<head>processing requirement or some other kind of phase handling.
The intended/specified behavior is that, like <script type="module" src="/app.js"></script>, such a <link> will cause "acquiring import maps" to flip to false, and thus prevent any import maps from being loaded. See https://wicg.github.io/import-maps/#integration-wait-for-import-maps
The upshot here, is that import maps should be before any modulepreloads. Allowing more flexibility here would require something similar to solving #248, e.g. https://github.com/guybedford/import-maps-extensions#lazy-loading-of-import-maps
The bug then seems to be that this works in the uncached case - that is, <link rel="modulepreload"> is not currently flipping the "acquiring import maps" state. Is it worth updating the bug to track this as an implementation bug?
Hmm, so I guess I'm not 100% clear on which cases were tested.
This is the case I understand you to be describing:
<link rel=modulepreload>something using bare specifiers, then<script type=importmap>, then<script type=module>the modulepreloaded thing:- Per-spec behavior: modulepreload flips the state, so importmap script will fire an
errorevent, and the module script will throw a TypeError when it fails to resolve the bare specifier (since the import map was never loaded). - Actual behavior: ???
- Per-spec behavior: modulepreload flips the state, so importmap script will fire an
Is there another case under discussion?
The issue is that the import map is still applying after the preload in the uncached case, resulting in no bare specifier error.
Actual behaviour is that the import map is never blocked - so the application behaves differently depending on which wins - the modulepreload or the actual module tag, which then differs between cached and uncached loads.
The fix should be to ensure that modulepreload does flip the state (which it isn't right now).
Got it, yeah, then we should make sure that is tracked on the implementation bug and is part of the fix @hiroshige-g is working on there.
@guybedford @domenic thanks for the discussion and references!
I didn't take into account that as per the spec, modulepreload is supposed to be able to fetch the entire module graph, not just the linked module itself. So this behavior is starting to make more sense.
Interestingly enough, I initially ran into this issue when attempting to use modulepreload link headers, as opposed to elements, to allow browsers to discover them as early as possible, and in anticipation for Cloudflare's 103 early hints support.
Import maps can only be specified inside the document in the current implementation, and es-module-shim isn't going to be able to polyfill header loading behavior, so it seems impossible to use modulepreload link headers in combination with import maps today.
Is this something that might be supported in the future when external import maps gets implemented? So we can add a link preload header for the import map before all the modulepreload headers?
As a separate discussion, do we feel the behavior for modulepreload to fetch the entire module graph is actually useful enough to block this use case with modulepreload headers + inlined import maps? modulepreload won't be able to flatten module loading waterfalls on its own, and it's meant to be used for modules that will be imported almost immediately, triggering the full module graph load anyways, so seems to be of very limited value. I suspect this might also have something to do with why it hasn't been implemented yet?
I didn't take into account that as per the spec, modulepreload is supposed to be able to fetch the entire module graph, not just the linked module itself. So this behavior is starting to make more sense.
It's not just that. It's that modulepreload is supposed to parse and compile the module, which requires doing import specifier resolution. (Even if it doesn't do module fetching.)
Import maps can only be specified inside the document in the current implementation, and es-module-shim isn't going to be able to polyfill header loading behavior, so it seems impossible to use modulepreload link headers in combination with import maps today.
Yep, sad but true.
Is this something that might be supported in the future when external import maps gets implemented? So we can add a link preload header for the import map before all the modulepreload headers?
Interesting idea. It would need to be special-cased, but I've learned that we have similar special-cases for CSP in early hints already, so it's not impossible.
As a separate discussion, do we feel the behavior for modulepreload to fetch the entire module graph is actually useful enough to block this use case with modulepreload headers + inlined import maps?
I have always felt it's useful because browsers which will fetch at least the dependent modules will be strictly faster than browsers which don't, i.e. browsers which wait until actual module execution (which could be much later).
No browser has yet prioritized this aspect of making module loading faster though. Perhaps because they'd prefer authors to do the work of flattening the module graph.
Ah, understood. Seems like the best we can do today is put modulepreload link elements in our documents, which is good enough to at least flatten the waterfall. This was the biggest blocker for using unbundled modules in prod for me in the past, and I've already been really happy with the results!
In the future it'd be amazing to be able to leverage external import maps, modulepreload headers and early hints to squeeze out even more performance (and improve cache granularity, since the import map is usually the only thing that changes in my html), but we're definitely getting into diminishing returns here.
Still, super excited about the future, and thank you all for the work on this stuff so far!
I believe the spec here does not need changes, and the Chromium bug was fixed including with web platform tests. So I will close this now. Let me know if I missed something and there's a reason to keep it open.