Skip to content

fix(jest-config): add mjs and cjs to default moduleFileExtensions config - #12578

Merged
SimenB merged 12 commits into
jestjs:mainfrom
F3n67u:moduleFileExtensions
Mar 29, 2022
Merged

fix(jest-config): add mjs and cjs to default moduleFileExtensions config#12578
SimenB merged 12 commits into
jestjs:mainfrom
F3n67u:moduleFileExtensions

Conversation

@F3n67u

@F3n67u F3n67u commented Mar 15, 2022

Copy link
Copy Markdown
Contributor

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

SimenB commented Mar 22, 2022

Copy link
Copy Markdown
Member

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

F3n67u commented Mar 22, 2022

Copy link
Copy Markdown
Contributor Author

We should add CJS as well 🙂

@SimenB done.

Comment thread CHANGELOG.md Outdated
{
exts: this._config.moduleFileExtensions.map(ext => `.${ext}`),
exts: this._config.moduleFileExtensions
.filter(ext => ext !== 'mjs')

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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.