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

Handle ERR_REQUIRE_ESM when reading configuration #9573

Merged
merged 4 commits into from
Feb 16, 2020

Conversation

azz
Copy link
Contributor

@azz azz commented Feb 15, 2020

Summary

#9431 added support for jest.config.mjs files.

This PR attempts to support jest.config.js files that are ES modules, when a project's package.json has "type": "module".

Current behaviour (Jest 25.1.0):

  • If config file ends with .mjs, do await import(configPath).
  • Otherwise, do require(configPath).

Proposed behaviour:

  • Do require(configPath)
  • If an ERR_REQUIRE_ESM is thrown, do await import(configPath).

This will leave the module detection logic to Node.js, in lieu of an API to query it (nodejs/node#49446).

It will be compatible with older versions of Node (8), and should be somewhat resilient to changes in the modules spec.

Test plan

I'm opening this PR as a discussion, so I haven't added any tests just yet.

There's already some discussion here.

@codecov-io
Copy link

codecov-io commented Feb 15, 2020

Codecov Report

Merging #9573 into master will decrease coverage by 0.01%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #9573      +/-   ##
==========================================
- Coverage   65.04%   65.03%   -0.02%     
==========================================
  Files         286      286              
  Lines       12140    12139       -1     
  Branches     3008     3008              
==========================================
- Hits         7897     7895       -2     
  Misses       3605     3605              
- Partials      638      639       +1
Impacted Files Coverage Δ
packages/jest-config/src/importEsm.ts 0% <ø> (ø)
...ges/jest-config/src/readConfigFileAndSetRootDir.ts 0% <0%> (ø) ⬆️
packages/expect/src/utils.ts 94.96% <0%> (-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 f47ade4...0a09601. Read the comment docs.

@azz azz requested a review from SimenB February 15, 2020 06:18
@SimenB
Copy link
Member

SimenB commented Feb 15, 2020

I think I read somewhere that this will print a warning to the console. Does it? If not this looks ideal 🙂

@azz
Copy link
Contributor Author

azz commented Feb 15, 2020

In my test script on Node 13.8 I get the warning:

(node:1) ExperimentalWarning: The ESM module loader is experimental.

But doesn't that happen with the code on master too? It will get logged the first time you do import() on an ES module.

@azz
Copy link
Contributor Author

azz commented Feb 15, 2020

test.cjs:

async function test() {
  let mod;
  try {
    mod = require('./config.js');
    console.log('require worked');
  } catch (e) {
    if (e.code === 'ERR_REQUIRE_ESM') {
      console.log('falling back to import()');
      mod = (await import('./config.js')).default;
    }
 }
 console.log(mod);
}

test();

config.js:

export default { foo: 'bar' };

package.json:

{"type":"module"}
$ docker run --rm -it -v "$(pwd):/tmp" -w /tmp node:13 node test
falling back to import()
(node:1) ExperimentalWarning: The ESM module loader is experimental.
{ foo: 'bar' }

@SimenB
Copy link
Member

SimenB commented Feb 15, 2020

Perfect, thanks for checking! 🙂 Just a test and we can merge this then 👍

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.

thank you!

@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.

4 participants