fix: isolate esm async import bug by eryue0220 · Pull Request #14397 · jestjs/jest

eryue0220 · 2023-08-09T10:30:48Z

Summary

Test plan

test locally

 e2e/__tests__/asyncESMImport.test.ts
  ✓ runs test with async ESM import (282 ms)

Test Suites: 1 passed, 1 total
Tests:       1 passed, 1 total
Snapshots:   1 passed, 1 total
Time:        0.514 s, estimated 1 s
Ran all test suites matching.

netlify · 2023-08-09T10:30:55Z

✅ Deploy Preview for jestjs ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	`a06645e`
🔍 Latest deploy log	https://app.netlify.com/sites/jestjs/deploys/64e353c77896880008faac97
😎 Deploy Preview	https://deploy-preview-14397--jestjs.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

mrazauskas · 2023-08-09T10:47:46Z

Looks right, but the Test Plan went missing (;

Is it possible to add an e2e test? Hm.. As I was mentioning in the issue, there is no need to have "type": "module" in package.json. Simply await import() an .mjs file and run Jest with --experimental-vm-modules. That can be done like here:

jest/e2e/__tests__/resolveAsync.test.ts

Lines 11 to 14 in 0fd5b1c

    
           test('runs test with native ESM', () => { 
        
             const {exitCode, stderr, stdout} = runJest('resolve-async', [], { 
        
               nodeOptions: '--experimental-vm-modules --no-warnings', 
        
             });

Or there is some reason, which I didn’t take into account?

eryue0220 · 2023-08-09T14:02:43Z

Looks right, but the Test Plan went missing (;

Is it possible to add an e2e test? Hm.. As I was mentioning in the issue, there is no need to have "type": "module" in package.json. Simply await import() an .mjs file and run Jest with --experimental-vm-modules. That can be done like here:

jest/e2e/__tests__/resolveAsync.test.ts

Lines 11 to 14 in 0fd5b1c

test('runs test with native ESM', () => {

const {exitCode, stderr, stdout} = runJest('resolve-async', [], {

nodeOptions: '--experimental-vm-modules --no-warnings',

});

Or there is some reason, which I didn’t take into account?

Sorry, I don't remember it. I'll fix them.

eryue0220 · 2023-08-09T16:39:09Z

Looks right, but the Test Plan went missing (;

Is it possible to add an e2e test? Hm.. As I was mentioning in the issue, there is no need to have "type": "module" in package.json. Simply await import() an .mjs file and run Jest with --experimental-vm-modules. That can be done like here:

jest/e2e/__tests__/resolveAsync.test.ts

Lines 11 to 14 in 0fd5b1c

test('runs test with native ESM', () => {

const {exitCode, stderr, stdout} = runJest('resolve-async', [], {

nodeOptions: '--experimental-vm-modules --no-warnings',

});

Or there is some reason, which I didn’t take into account?

I added test plan.

But the

eryue0220 · 2023-08-09T16:41:29Z

I updated the test plan and the e2e test case.

mrazauskas

Thanks! It is looking good.

I checked out and tried out on different versions of Node locally. Could you push the suggested changes to see if that also works in CI?

mrazauskas · 2023-08-09T17:35:17Z

+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+import {jest} from '@jest/globals';


Suggested change

import {jest} from '@jest/globals';

This import could not remove, because the next code will use the api jest. isolateModulesAsync

mrazauskas

Thanks. Seems like CI is happy.

One setup detail could be polished. I left an inline comment.

mrazauskas

Sorry, I was going through again and notice few more details. Could you look at them, please?

mrazauskas · 2023-08-11T03:22:19Z

+    const registry = this._isolatedESMModuleRegistry
+      ? this._isolatedESMModuleRegistry
+      : this._esmoduleRegistry;


Hm.. Isn’t it enough to use the same _isolatedMockRegistry? Did you try that?

It was my idea about missing _isolatedESMModuleRegistry, but it can I was wrong. Here we can see that _isolatedMockRegistry was simply not used while loading ES modules. Sorry about slow thinking. Could you try to _isolatedMockRegistry everywhere?

Of course, it is possible. I originally thought it was intentionally differentiated because I saw that there was already an existing _isolatedModuleRegistry, moreover, I think it seems more appropriate.

mrazauskas

Yes! Simple and clear. Thanks for collaboration!

(CI fails are not related.)

eryue0220 · 2023-08-18T15:57:42Z

can this pr be merged, if no other problems ? cc @SimenB @mrazauskas

SimenB

wonderful, thanks!

SimenB · 2023-08-21T12:42:41Z

https://github.com/jestjs/jest/releases/tag/v29.6.3

github-actions · 2023-09-21T00:07:51Z

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.

fix: isolate esm async import bug

3df4711

eryue0220 added 2 commits August 10, 2023 00:29

fix: add test plan

e6ba77b

chore: update pkg.json && readme

a53ffa5

eryue0220 closed this Aug 9, 2023

eryue0220 reopened this Aug 9, 2023

mrazauskas reviewed Aug 9, 2023

View reviewed changes

Comment thread e2e/resolve-async/package.json

mrazauskas suggested changes Aug 9, 2023

View reviewed changes

fix: code review

54ff86e

mrazauskas reviewed Aug 10, 2023

View reviewed changes

Comment thread e2e/async-esm-import/package.json Outdated

fix: e2e test update

08f9bb8

mrazauskas reviewed Aug 10, 2023

View reviewed changes

Comment thread e2e/async-esm-import/package.json Outdated

chore: remote config

3710da6

mrazauskas reviewed Aug 10, 2023

View reviewed changes

Comment thread packages/jest-runtime/src/index.ts

fix: cr problem

60f9f59

mrazauskas suggested changes Aug 11, 2023

View reviewed changes

eryue0220 added 3 commits August 12, 2023 12:52

chore: refactor file

3a9d0a6

fix: e2e test

e12f6ab

fix: remove property

3bbf1ba

mrazauskas approved these changes Aug 12, 2023

View reviewed changes

eryue0220 added 2 commits August 12, 2023 15:55

fix: resolve conflict

ce55918

fix: resolve conflict

e959b07

SimenB added 2 commits August 21, 2023 14:07

Merge branch 'main' into fix/issue-14387

d218ca0

move changelog entry

6f7d689

SimenB approved these changes Aug 21, 2023

View reviewed changes

Merge branch 'main' into fix/issue-14387

a06645e

SimenB merged commit 58e8491 into jestjs:main Aug 21, 2023

github-actions Bot locked as resolved and limited conversation to collaborators Sep 21, 2023

Uh oh!

Conversation

eryue0220 commented Aug 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

netlify Bot commented Aug 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for jestjs ready!

Uh oh!

mrazauskas commented Aug 9, 2023

Uh oh!

eryue0220 commented Aug 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eryue0220 commented Aug 9, 2023

Uh oh!

eryue0220 commented Aug 9, 2023

Uh oh!

Uh oh!

mrazauskas left a comment

Choose a reason for hiding this comment

Uh oh!

mrazauskas Aug 9, 2023

Choose a reason for hiding this comment

Uh oh!

eryue0220 Aug 10, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mrazauskas left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mrazauskas left a comment

Choose a reason for hiding this comment

Uh oh!

mrazauskas Aug 11, 2023

Choose a reason for hiding this comment

Uh oh!

eryue0220 Aug 12, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mrazauskas left a comment

Choose a reason for hiding this comment

Uh oh!

eryue0220 commented Aug 18, 2023

Uh oh!

SimenB left a comment

Choose a reason for hiding this comment

Uh oh!

SimenB commented Aug 21, 2023

Uh oh!

github-actions Bot commented Sep 21, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

eryue0220 commented Aug 9, 2023 •

edited

Loading

netlify Bot commented Aug 9, 2023 •

edited

Loading

eryue0220 commented Aug 9, 2023 •

edited

Loading