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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -50,6 +50,7 @@
- `[jest-circus, @jest/types]` Disallow undefined value in `TestContext` type ([#12507](https://github.com/facebook/jest/pull/12507))
- `[jest-config]` Correctly detect CI environment and update snapshots accordingly ([#12378](https://github.com/facebook/jest/pull/12378))
- `[jest-config]` Pass `moduleTypes` to `ts-node` to enforce CJS when transpiling ([#12397](https://github.com/facebook/jest/pull/12397))
- `[jest-config]` [**BREAKING**] Add `mjs` and `cjs` to default `moduleFileExtensions` config ([12578](https://github.com/facebook/jest/pull/12578))
- `[jest-config, jest-haste-map]` Allow searching for tests in `node_modules` by exposing `retainAllFiles` ([#11084](https://github.com/facebook/jest/pull/11084))
- `[jest-core]` [**BREAKING**] Exit with status `1` if no tests are found with `--findRelatedTests` flag ([#12487](https://github.com/facebook/jest/pull/12487))
- `[jest-each]` `%#` is not replaced with index of the test case ([#12517](https://github.com/facebook/jest/pull/12517))
Expand Down
2 changes: 1 addition & 1 deletion docs/Configuration.md
Expand Up @@ -571,7 +571,7 @@ An array of directory names to be searched recursively up from the requiring mod

### `moduleFileExtensions` \[array<string>]

Default: `["js", "jsx", "ts", "tsx", "json", "node"]`
Default: `["js", "mjs", "cjs", "jsx", "ts", "tsx", "json", "node"]`

An array of file extensions your modules use. If you require modules without specifying a file extension, these are the extensions Jest will look for, in left-to-right order.

Expand Down
2 changes: 2 additions & 0 deletions e2e/__tests__/__snapshots__/showConfig.test.ts.snap
Expand Up @@ -30,6 +30,8 @@ exports[`--showConfig outputs config info and exits 1`] = `
],
"moduleFileExtensions": [
"js",
"mjs",
"cjs",
"jsx",
"ts",
"tsx",
Expand Down
9 changes: 9 additions & 0 deletions e2e/__tests__/__snapshots__/testMatch.test.ts.snap
@@ -0,0 +1,9 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`testMatch should able to match file with cjs and mjs extension 1`] = `
"Test Suites: 2 passed, 2 total
Tests: 2 passed, 2 total
Snapshots: 0 total
Time: <<REPLACED>>
Ran all test suites."
`;
16 changes: 16 additions & 0 deletions e2e/__tests__/testMatch.test.ts
@@ -0,0 +1,16 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

import {extractSummary} from '../Utils';
import runJest from '../runJest';

it('testMatch should able to match file with cjs and mjs extension', () => {
const result = runJest('test-match');
expect(result.exitCode).toBe(0);
const {summary} = extractSummary(result.stderr);
expect(summary).toMatchSnapshot();
});
7 changes: 7 additions & 0 deletions e2e/test-match/package.json
@@ -0,0 +1,7 @@
{
"jest": {
"testMatch": [
"**/test-suites/*.?js"
]
}
}
10 changes: 10 additions & 0 deletions e2e/test-match/test-suites/sample-suite.mjs
@@ -0,0 +1,10 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

test('mjs extension', () => {
expect(1).toBe(1);
});
10 changes: 10 additions & 0 deletions e2e/test-match/test-suites/sample-suite2.cjs
@@ -0,0 +1,10 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

test('cjs extension', () => {
expect(1).toBe(1);
});
Expand Up @@ -191,6 +191,8 @@ module.exports = {
// An array of file extensions your modules use
// moduleFileExtensions: [
// "js",
// "mjs",
// "cjs",
// "jsx",
// "ts",
// "tsx",
Expand Down
11 changes: 10 additions & 1 deletion packages/jest-config/src/Defaults.ts
Expand Up @@ -44,7 +44,16 @@ const defaultOptions: Config.DefaultOptions = {
maxConcurrency: 5,
maxWorkers: '50%',
moduleDirectories: ['node_modules'],
moduleFileExtensions: ['js', 'jsx', 'ts', 'tsx', 'json', 'node'],
moduleFileExtensions: [
'js',
'mjs',
'cjs',
'jsx',
'ts',
'tsx',
'json',
'node',
],
moduleNameMapper: {},
modulePathIgnorePatterns: [],
noStackTrace: false,
Expand Down
11 changes: 10 additions & 1 deletion packages/jest-config/src/ValidConfig.ts
Expand Up @@ -74,7 +74,16 @@ const initialOptions: Config.InitialOptions = {
maxConcurrency: 5,
maxWorkers: '50%',
moduleDirectories: ['node_modules'],
moduleFileExtensions: ['js', 'json', 'jsx', 'ts', 'tsx', 'node'],
moduleFileExtensions: [
'js',
'mjs',
'cjs',
'json',
'jsx',
'ts',
'tsx',
'node',
],
moduleNameMapper: {
'^React$': '<rootDir>/node_modules/react',
},
Expand Down
2 changes: 2 additions & 0 deletions packages/jest-config/src/__tests__/normalize.test.ts
Expand Up @@ -1668,6 +1668,8 @@ describe('moduleFileExtensions', () => {

expect(options.moduleFileExtensions).toEqual([
'js',
'mjs',
'cjs',
'jsx',
'ts',
'tsx',
Expand Down
5 changes: 4 additions & 1 deletion packages/jest-transform/src/ScriptTransformer.ts
Expand Up @@ -759,7 +759,10 @@ class ScriptTransformer {
}
},
{
exts: this._config.moduleFileExtensions.map(ext => `.${ext}`),
// Exclude `mjs` extension when addHook because pirates don't support hijack es module
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.

.map(ext => `.${ext}`),
ignoreNodeModules: false,
matcher: filename => {
if (transforming) {
Expand Down