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

Fix immediately added dynamic import maps #123

Merged
merged 4 commits into from May 14, 2021

Conversation

vovacodes
Copy link
Collaborator

@vovacodes vovacodes commented May 14, 2021

This draft adds a failing test (http://localhost:8080/test/dynamic-import-map-shim.html) for the scenario when an import map is inserted dynamically from JS and importShim() that is trying to import a bare module specifier defined in that dynamic import map is called right after the insertion.

In the state after the first commit the code works in Safari and prints the following to the console

image

but it fails in Chrome with printing the following:

image

The second commit fixes it.

@vovacodes vovacodes requested a review from guybedford May 14, 2021 08:08
@vovacodes vovacodes force-pushed the fix/immediately-added-dynamic-import-map branch from 2fd7510 to 637fc6e Compare May 14, 2021 08:31
@vovacodes vovacodes marked this pull request as ready for review May 14, 2021 08:33
Copy link
Owner

@guybedford guybedford left a comment

Choose a reason for hiding this comment

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

Great to have a test. Let me know if I can help look into it further? I don't currently have a mac but could test on my iphone at least.

src/es-module-shims.js Show resolved Hide resolved
src/es-module-shims.js Show resolved Hide resolved
@vovacodes vovacodes requested a review from guybedford May 14, 2021 10:25
@vovacodes
Copy link
Collaborator Author

@guybedford I think I'm ready to merge it if my response to your comments makes sense to you.

@vovacodes vovacodes force-pushed the fix/immediately-added-dynamic-import-map branch from 637fc6e to 6f594e6 Compare May 14, 2021 10:34
src/es-module-shims.js Outdated Show resolved Hide resolved
@vovacodes vovacodes force-pushed the fix/immediately-added-dynamic-import-map branch from b9ecc18 to c7bbb01 Compare May 14, 2021 13:18
@vovacodes vovacodes force-pushed the fix/immediately-added-dynamic-import-map branch from c7bbb01 to e082019 Compare May 14, 2021 13:21
@guybedford guybedford merged commit f6f7bda into master May 14, 2021
@guybedford guybedford deleted the fix/immediately-added-dynamic-import-map branch May 14, 2021 13:22
@vovacodes
Copy link
Collaborator Author

@guybedford do you mind publishing a new version?

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.

None yet

2 participants