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

feat: support importing CommonJS from ESM files #9842

Merged
merged 5 commits into from Apr 19, 2020

Conversation

SimenB
Copy link
Member

@SimenB SimenB commented Apr 19, 2020

Summary

Luckily importing CJS from ESM should use the normal require caching mechanisms, so we can just hook into the code we already have for this.

Test plan

E2E test expanded with new test cases.

@codecov-io
Copy link

Codecov Report

Merging #9842 into master will decrease coverage by 0.07%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #9842      +/-   ##
==========================================
- Coverage   64.52%   64.45%   -0.08%     
==========================================
  Files         290      290              
  Lines       12325    12336      +11     
  Branches     3048     3052       +4     
==========================================
- Hits         7953     7951       -2     
- Misses       3732     3744      +12     
- Partials      640      641       +1     
Impacted Files Coverage Δ
packages/jest-runtime/src/index.ts 60.00% <0.00%> (-1.30%) ⬇️
packages/expect/src/utils.ts 94.96% <0.00%> (-1.26%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3681cca...1c6555b. Read the comment docs.

initializeImportMeta(meta: ImportMeta) {
meta.url = pathToFileURL(modulePath).href;
},
});

this._esmoduleRegistry.set(cacheKey, module);

await module.link((specifier: string, referencingModule: VMModule) =>
Copy link
Member Author

@SimenB SimenB Apr 19, 2020

Choose a reason for hiding this comment

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

actually quite the bugfix moving this in here, dynamic import of ESM didn't work unless it was cached already 😀 (modules must be linked and evaluated before being returned)

@SimenB SimenB merged commit 8b70b47 into jestjs:master Apr 19, 2020
@SimenB SimenB deleted the module-import branch April 19, 2020 21:38
@SimenB SimenB mentioned this pull request Apr 19, 2020
21 tasks
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants