Skip to content

feat: hard multi-version protections#486

Merged
guybedford merged 7 commits intomainfrom
protect-double-load
Apr 26, 2025
Merged

feat: hard multi-version protections#486
guybedford merged 7 commits intomainfrom
protect-double-load

Conversation

@guybedford
Copy link
Owner

@guybedford guybedford commented Apr 25, 2025

Adds the hard version protections suggested in #484 - freezing the importShim object on the global and also freezing its properties.

I would still like to verify before landing this:

  1. The shim mode switch is definitely necessary. I would have thought the ep markers would have been conflict-avoiding here therefore this would be unnecessary.
  2. The defineValue(self, '_d', undefined) thing - what version this is deconflicting with and why.

@guybedford guybedford force-pushed the protect-double-load branch from 905704b to 48e3227 Compare April 25, 2025 20:50
@guybedford
Copy link
Owner Author

Another thing we could do here to help solve the race issue in future is to support a new init option, version, where when set, only that exact version of es-module-shims is permitted to register on the page. I've added an experimental commit to this effect, @frandiox would be interested to get your feedback on that as well.

Copy link

@frandiox frandiox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for tackling this!

The shim mode switch is definitely necessary. I would have thought the ep markers would have been conflict-avoiding here therefore this would be unnecessary.

This seems needed in some versions indeed. For example, 2.0.0 or 1.4.0 + Firefox

The defineValue(self, '_d', undefined) thing - what version this is deconflicting with and why.

I found about the _d here, which is code included in versions around 2.0.0 (removed in 2.0.3), including earlier versions like 1.10.
It's hard to reproduce but it happens in Safari with these versions sometimes. Changing the _d seemed to disable the checks in these versions. I'm not 100% sure about the mechanism, but I think that assigning undefined in _d propagates the undefined value to dynamicImport, and thanks to that it gets overwritten by this custom function 🤔 -- I'm not sure why the original u=>import(u) is problematic but it seems to create issues indeed (maybe it can't be rewritten by the first loaded polyfill? Unsure). It makes me suspect there might be some mixed behavior between the 2 loaded polyfills but I don't really know.

I've pushed a branch to my test repo with the patched polyfill, in case you want to test it: https://github.com/frandiox/importmap-polyfill-test/tree/fd-patch-test -- you can play with removing async attribute as well to ensure order.

Hope this is helpful!

guybedford and others added 3 commits April 25, 2025 21:34
@guybedford
Copy link
Owner Author

Releasing in 2.4.0 now.

@guybedford guybedford merged commit 33c92b6 into main Apr 26, 2025
9 of 10 checks passed
@guybedford guybedford deleted the protect-double-load branch April 26, 2025 04:48
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

Successfully merging this pull request may close these issues.

2 participants