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(jest-config): add mjs and cjs to default moduleFileExtensions config #12578

Merged
merged 12 commits into from
Mar 29, 2022

Conversation

F3n67u
Copy link
Contributor

@F3n67u F3n67u commented Mar 15, 2022

Summary

fix #12237

Test plan

test locally, see below:

node ~/dev/jest/packages/jest-cli/bin/jest.js         
 PASS   Sample Test Suite  tests/test-suites/sample-suite.mjs
  ✓ hello world (2 ms)

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

@F3n67u F3n67u marked this pull request as draft March 15, 2022 14:30
@F3n67u F3n67u force-pushed the moduleFileExtensions branch from 4b93f32 to e010106 Compare March 17, 2022 00:39
@F3n67u F3n67u marked this pull request as ready for review March 17, 2022 14:17
@F3n67u F3n67u marked this pull request as draft March 19, 2022 10:17
@SimenB
Copy link
Member

SimenB commented Mar 22, 2022

We should add CJS as well 🙂

@F3n67u F3n67u marked this pull request as ready for review March 22, 2022 11:51
@F3n67u F3n67u changed the title fix(jest-config): add mjs to default moduleFileExtensions config fix(jest-config): add mjs and cjs to default moduleFileExtensions config Mar 22, 2022
@F3n67u
Copy link
Contributor Author

F3n67u commented Mar 22, 2022

We should add CJS as well 🙂

@SimenB done.

CHANGELOG.md Outdated Show resolved Hide resolved
@@ -759,7 +759,9 @@ class ScriptTransformer {
}
},
{
exts: this._config.moduleFileExtensions.map(ext => `.${ext}`),
exts: this._config.moduleFileExtensions
.filter(ext => ext !== 'mjs')
Copy link
Member

Choose a reason for hiding this comment

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

why this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

transform.test.ts is fail when including mjs in the extension list.

$ ./packages/jest/bin/jest.js '/Users/feng/dev/jest/e2e/__tests__/transform.test.ts'
transform-esm-runner
      ✕ runs test with native ESM (618 ms)
    transform-esm-testrunner
      ✕ runs test with native ESM (868 ms)

  ● on node >=12.17.0 › transform-esm-runner › runs test with native ESM


          Can't parse JSON.
          ERROR: SyntaxError Unexpected end of JSON input
          STDOUT: 
          STDERR: /Users/feng/dev/jest/e2e/transform/transform-esm-runner/runner.mjs:8
    import testResult from '@jest/test-result';
    ^^^^^^

    SyntaxError: Cannot use import statement outside a module

      146 |     };
      147 |   } catch (e: any) {
    > 148 |     throw new Error(
          |           ^
      149 |       `
      150 |       Can't parse JSON.
      151 |       ERROR: ${e.name} ${e.message}

      at Module._compile (node_modules/pirates/lib/index.js:135:24)
          
      at json (e2e/runJest.ts:148:11)
      at Object.<anonymous> (e2e/__tests__/transform.test.ts:326:30)

  ● on node >=12.17.0 › transform-esm-testrunner › runs test with native ESM

    expect(received).toMatch(expected)

    Expected pattern: /PASS/
    Received string:  "FAIL __tests__/add.test.js
      ● Test suite failed to run·
        /Users/feng/dev/jest/e2e/transform/transform-esm-testrunner/test-runner.mjs:7
        import testResult from '@jest/test-result';
        ^^^^^^·
        SyntaxError: Cannot use import statement outside a module·
          at Module._compile (../../../node_modules/pirates/lib/index.js:135:24)·
    Test Suites: 1 failed, 1 total
    Tests:       0 total
    Snapshots:   0 total
    Time:        0.258 s
    Ran all test suites."

      344 |       });
      345 |
    > 346 |       expect(stderr).toMatch(/PASS/);
          |                      ^
      347 |       expect(json.success).toBe(true);
      348 |       expect(json.numPassedTests).toBe(1);
      349 |     });

      at Object.toMatch (e2e/__tests__/transform.test.ts:346:22)

Test Suites: 1 failed, 1 total
Tests:       2 failed, 20 passed, 22 total
Snapshots:   5 passed, 5 total
Time:        42.588 s, estimated 59 s
Ran all test suites matching /\/Users\/feng\/dev\/jest\/e2e\/__tests__\/transform.test.ts/i.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

pirate just hijack require, .mjs is es module, so we should filter out .mjs extension when we addHook

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if we override Module._extensions[".mjs"], then require('test-runner.mjs') will not throw ERR_REQUIRE_ESM error but instead will an Cannot use import statement outside a module error will throw when pirates call Module._compile.

nodejs source code:

https://github.com/nodejs/node/blob/8a96ff7e545041dbf08483b4d512eefb488f0479/lib/internal/modules/cjs/loader.js#L978-L980

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added some comments to make the filter mjs extension's intention more clearer.

@F3n67u F3n67u force-pushed the moduleFileExtensions branch from 8be5ff0 to be8ad01 Compare March 29, 2022 01:28
@jestjs jestjs deleted a comment from 337Gslime Mar 29, 2022
Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

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

thanks!

@SimenB SimenB merged commit daf583c into jestjs:main Mar 29, 2022
@F3n67u F3n67u deleted the moduleFileExtensions branch March 29, 2022 12:01
@SimenB
Copy link
Member

SimenB commented Apr 5, 2022

@github-actions
Copy link

github-actions bot commented May 6, 2022

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 6, 2022
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.

[Bug]: ES6 Test Match in configuration file ignoring .mjs extension
3 participants