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

Loading .cjs config files on Node 13 #9086

Closed
the-spyke opened this issue Oct 24, 2019 · 22 comments · Fixed by #9291
Closed

Loading .cjs config files on Node 13 #9086

the-spyke opened this issue Oct 24, 2019 · 22 comments · Fixed by #9291

Comments

@the-spyke
Copy link
Contributor

🚀 Feature Proposal

If you have Node 13 and jest.config.js file in a package with type=module then Node will emit a warning:

(node:8906) Warning: require() of ES modules is not supported.
require() of /home/spyke/Projects/undercut/packages/undercut/jest.config.js from /home/spyke/Projects/undercut/node_modules/jest-config/build/readConfigFileAndSetRootDir.js is an ES module file as it is a .js file whose nearest parent package.json contains "type": "module" which defines all .js files in that package scope as ES modules.
Instead rename jest.config.js to end in .cjs, change the requiring code to use import(), or remove "type": "module" from /home/spyke/Projects/undercut/packages/undercut/package.json.

In case of manually specifying the config fole with --config jest.config.cjs it fails with an error:

The --config option requires a JSON string literal, or a file path with a .js or .json extension.
Example usage: jest --config ./jest.config.js

Using ES Modules syntax for config file gives an error too:

SyntaxError: Unexpected token 'export'

The issue is the same as babel/babel#10595. .cjs is mandatory, but being able to load an ESM from .js or .mjs is nice to have too.

Motivation

Support ES Modules workflows in Node13+

Example

Configuring Jest in packages with type=module and config file named jest.config.cjs

Pitch

@kirill-konshin
Copy link

FYI nodejs/node#30694 (comment)

@SimenB
Copy link
Member

SimenB commented Feb 13, 2020

Sorry, I don't follow. What's the issue in Jest?

@kirill-konshin
Copy link

kirill-konshin commented Feb 13, 2020

The problem is that after upgrade of Node from 12.15 to 12.16 warning become an error.

I have a type=module, jest.config.js and .babelrc.js in my setup, it worked with warning, now it does not. Just putting a reference here for anyone who'll look for more info just in case.

So in essence this ticket also applies to Node 12, not just 13 as mentioned in title.

@SimenB
Copy link
Member

SimenB commented Feb 13, 2020

Aha! Gotcha, thanks for clarifying. You can rename the file to .cjs if you're on Jest 25 as a fix (or .mjs if you want). I think babel also supports .cjs

@kirill-konshin
Copy link

kirill-konshin commented Feb 13, 2020

It will break differently, unfortunately.

If I rename jest.config.cjs:

@service-web/commonHelpers: $ rc-jest
@service-web/commonHelpers: FAIL src/common/utils.spec.ts
@service-web/commonHelpers:   ● Test suite failed to run
@service-web/commonHelpers:     /xxx/src/commonHelpers/.babelrc.js: Error while loading config - Must use import to load ES Module: /Users/dis/Sites/web/src/commonHelpers/.babelrc.js
@service-web/commonHelpers:     require() of ES modules is not supported.
@service-web/commonHelpers:     require() of /Users/dis/Sites/web/src/commonHelpers/.babelrc.js from /Users/dis/Sites/web/node_modules/@babel/core/lib/config/files/configuration.js is an ES module file as it is a .js file whose nearest parent package.json contains "type": "module" which defines all .js files in that package scope as ES modules.
@service-web/commonHelpers:     Instead rename .babelrc.js to end in .cjs, change the requiring code to use import(), or remove "type": "module" from /Users/dis/Sites/web/src/commonHelpers/package.json.
@service-web/commonHelpers:       at ../../node_modules/@babel/core/lib/config/files/configuration.js:201:26
@service-web/commonHelpers:       at cachedFunction (../../node_modules/@babel/core/lib/config/caching.js:33:19)
@service-web/commonHelpers:       at readConfig (../../node_modules/@babel/core/lib/config/files/configuration.js:173:56)
@service-web/commonHelpers:       at ../../node_modules/@babel/core/lib/config/files/configuration.js:105:24
@service-web/commonHelpers:           at Array.reduce (<anonymous>)

Then if I rename .babelrc.cjs I get:

@service-web/commonHelpers: FAIL src/common/utils.spec.ts
@service-web/commonHelpers:   ● Test suite failed to run
@service-web/commonHelpers:     /xxx/src/commonHelpers/src/common/utils.spec.ts:1
@service-web/commonHelpers:     ({"Object.<anonymous>":function(module,exports,require,__dirname,__filename,global,jest){import { getDispositionFileName } from './utils';
@service-web/commonHelpers:                                                                                              ^^^^^^
@service-web/commonHelpers:     SyntaxError: Cannot use import statement outside a module
@service-web/commonHelpers:       at ScriptTransformer._transformAndBuildScript (../../node_modules/@jest/transform/build/ScriptTransformer.js:537:17)
@service-web/commonHelpers:       at ScriptTransformer.transform (../../node_modules/@jest/transform/build/ScriptTransformer.js:579:25)

... the problem is that our previous release (for which we're doing patches now) was on Jest 24... And we cannot do major version upgrade that easily due to policies etc etc.

@SimenB
Copy link
Member

SimenB commented Feb 13, 2020

That second one looks weird... Babel supports .cjs from version 7.7.0 (https://github.com/babel/babel/releases/tag/v7.7.0), is that on an older version?

That won't help if you're stuck on Jest 24 though, as Jest won't recognize the config file... If your config file is just JSON you use that, or do some fancy hacks like putting your config in a preset in another module and load that in Jest (which should circumvent the package.json check for type). Completely untested, but maybe moving the jest config file to a directory with its own dummy package.json works?

@the-spyke
Copy link
Contributor Author

the-spyke commented Feb 13, 2020

@kirill-konshin This is an issue with your setup, probably some glob/regexs in config files are broken because of changed extensions. I'm using Node 13.8.0 and having no errors within undercut repo. As you can see both Babel and Jest configs are .cjs files. You can try by yourself:

yarn
yarn test

@kirill-konshin
Copy link

kirill-konshin commented Feb 13, 2020

@SimenB you're right, our babel in that outdated branch is 7.4.3.
@the-spyke we have quite an old setup at the moment.

So far we just downgraded to Node 12.15... I wonder how come such breaking change made its way to minor release.

Thank you for support!

@the-spyke
Copy link
Contributor Author

the-spyke commented Feb 13, 2020

@kirill-konshin I'm not sure if experimental features are a part of stable API. Also, it isn't really a change, because importing a file with require from ESM context shouldn't have been working in the first place failing require is undefined error.

@kirill-konshin
Copy link

@the-spyke somehow it worked on Node 12.15. It produced warnings, but code worked fine.

@the-spyke
Copy link
Contributor Author

@kirill-konshin Yeah, because it was experimental and did some things not as it should. That's why there was a warning: when implementation of ESM finishes require, __dirname, etc. will be undefined.

@kirill-konshin
Copy link

The most frustrating about this experimental feature is the transition from "no warning" to "warning" and then to "error" all within minor release...

@emma-borhanian
Copy link

@the-spyke

I'm using Node 13.8.0 and having no errors within undercut repo. As you can see both Babel and Jest configs are .cjs files. You can try by yourself:

If I run yarn jest --config jest.config.cjs instead of just yarn jest in your repo it fails with:

The --config option requires a JSON string literal, or a file path with a .js or .json extension.

This directly contradicts the jest documentation:

Jest's configuration can be defined in the package.json file of your project, or through a jest.config.js file or through the --config <path/to/file.js|cjs|mjs|json> option.

@SimenB
Copy link
Member

SimenB commented Feb 15, 2020

@emma-borhanian with jest 25? cjs and mjs support landed in 25 (in theory)

@emma-borhanian
Copy link

emma-borhanian commented Feb 16, 2020

@SimenB Yup, jest 25.

Maybe you missed a spot?
https://github.com/facebook/jest/blob/v25.1.0/packages/jest-cli/src/cli/args.ts#L55

  if (
    argv.config &&
    !isJSONString(argv.config) &&
    !argv.config.match(/\.js(on)?$/)
  ) {
    throw new Error(
      'The --config option requires a JSON string literal, or a file path with a .js or .json extension.\n' +
        'Example usage: jest --config ./jest.config.js',
    );
  }

undercut test log: https://gist.github.com/emma-borhanian/94a233217b2f8551157291aa3adcc956

emma-borhanian added a commit to emma-borhanian/mocha that referenced this issue Feb 16, 2020
This allows for `"type": "module"` in package.json while also using a
mocha config file, which was previously impossible.
See: https://nodejs.org/api/esm.html

Compare to jestjs/jest#9086
@the-spyke
Copy link
Contributor Author

@emma-borhanian Indeed, I somehow forget to patch --config option in CLI in #9291. But as you can see, Jest 25 loads the .cjs config by itself without using --config option. I'll submit a fix for the --config soon, sorry for inconvenience.

@SimenB
Copy link
Member

SimenB commented Feb 16, 2020

Aha, good catch @emma-borhanian!

@bouchja1
Copy link

Hi guys, this is really nice feature for future versions of Node. But I still have one problem with this. My script for the test is defined as:

"test": "jest --runInBand --forceExit --detectOpenHandles test/integration"

Running on Node v13.9.0

Then I have jest.config.cjs file with the setup:

module.exports = {
    coveragePathIgnorePatterns: ["/node_modules/"],
    transform: {
        "^.+\\.mjs$": "babel-jest",
    },
};

And babel.config.json as:

{
  "presets": ["@babel/preset-env"]
}

Package versions are updated:
"babel-jest": "^25.1.0"
"jest-cli": "^25.1.0"
"@babel/core": "^7.8.4"
"@babel/preset-env": "^7.8.4"

But I am still getting the error below when I run my tests...

_/Volumes/FjoogaDEV/projects/fjooga-api/test/unit/Organisation.test.js:1
({"Object.":function(module,exports,require,__dirname,__filename,global,jest){import { Container } from "typedi";
^^^^^^

SyntaxError: Cannot use import statement outside a module_

What am I doing wrong? Thank you in advance guys.

@the-spyke
Copy link
Contributor Author

@bouchja1 Your test file has .js extension, but Jest transformer is set to .mjs extension.

@bouchja1
Copy link

bouchja1 commented Feb 21, 2020

@bouchja1 Your test file has .js extension, but Jest transformer is set to .mjs extension.

That could be a problem. I have renamed it to *.mjs (and added

    moduleFileExtensions: [
       ....
        "mjs",
    ],

to jest.config.cjs but it says "No tests found, exiting with code 0" now... This is that test and I think it is written correctly.

import { Container } from "typedi";
import testApp from "../testApp";

describe("Test Organisations Service", () => {
    let organisationsService;

    beforeAll(async () => {
        await testApp();
        organisationsService = Container.get("organisationService");
    });

    test("Should something", async () => {
        expect(true).toBe(true);
    });
});

EDIT: I am able to run the test after adding this to jest.config.cjs...

testMatch: [
    "**/*.test.mjs"
  ] 

@the-spyke
Copy link
Contributor Author

@bouchja1 As you've renamed tests to .mjs I bet testMatch expression stopped working.

In general, you shouldn't rename files into .mjs if they are already ESMs and using Babel. The idea is that you set type="module" in your package.json and the entire package becomes ESM. .cjs needed for files loaded by tools like Jest that use require() internally. In this case you run a Node application (Jest) inside a ESM package folder, where Node thinks that every .js file is a ESM. And you can't require a ESM by design. To fix this you need to manually specify some files that aren't ESMs. Like renaming jest.config.js into jest.config.cjs.

@github-actions
Copy link

This issue 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 a pull request may close this issue.

5 participants