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

How to run jest when using es6 packages in node_modules #5241

Closed
nerfologist opened this issue Oct 2, 2018 · 11 comments

Comments

Projects
None yet
6 participants
@nerfologist
Copy link

commented Oct 2, 2018

Is this a bug report?

Not sure. More of a configuration issue I guess.

TLDR

Trying out the feature described in #1125 which should allow the usage of libraries distributed as es6 modules in an app create with create-react-app, the app runs fine but the Jest test suite will not.

How to reproduce

  • create a new create-react-app application npx create-react-app jest-es6-modules
  • in the new application, yarn add lodash-es
  • modify src/App.js adding any named import, such as import { isEmpty } from 'lodash-es'
  • run yarn start (everything works)
  • run yarn test

Here's the Jest error, related to Jest not babel-transforming the modules in es6 format:

  ● Test suite failed to run

    Jest encountered an unexpected token

    This usually means that you are trying to import a file which Jest cannot parse, e.g. it's not plain JavaScript.

    [...]

    Details:

    /Users/nerfologist/test/jest-es6-modules/node_modules/lodash-es/lodash.js:10
    export { default as add } from './add.js';
    ^^^^^^

    SyntaxError: Unexpected token export

      1 | import React, {Component} from 'react';
      2 | import logo from './logo.svg';
    > 3 | import {isEmpty} from 'lodash-es';
        | ^
      4 | import './App.css';
      5 |
      6 | class App extends Component {

      at ScriptTransformer._transformAndBuildScript (node_modules/jest-runtime/build/script_transformer.js:403:17)
      at Object.<anonymous> (src/App.js:3:1)

Test Suites: 1 failed, 1 total
Tests:       0 total
Snapshots:   0 total
Time:        1.279s
Ran all test suites related to changed files.

Question

What is the best/recommended way to have Jest transform one or more of these node_modules?

I'm thinking of adding a Jest configuration containing the transformIgnorePatterns directive.

Thanks in advance!

@Timer

This comment has been minimized.

Copy link
Collaborator

commented Oct 2, 2018

Hm, this is unfortunate that lodash ships separate packages for ESM and CommonJS. They should be under the same package.

Typically the way we handle this is by saying speak with the package publisher, or in this case, just use lodash. I can see the appeal in wanting to use lodash-es -- I wonder if we can get a version of lodash published where main points to CommonJS and the module/jsmain:next field(s) point to ESM. Thus having one package with proper support for Node and package bundlers.

@Timer

This comment has been minimized.

Copy link
Collaborator

commented Oct 2, 2018

Looks like @jdalton wasn't fond of this idea in lodash/lodash#3871. I wonder what the best solution is here.

@edmorley

This comment has been minimized.

Copy link

commented Oct 2, 2018

An alternative fix would be to remove the modules: false from the dependencies-specific Babel config here:

// Do not transform modules to CJS
modules: false,

...which would make @babel/preset-env transform them to CJS, like happens already for the main Babel configuration when NODE_ENV === 'test':

isEnvTest && [
// ES features necessary for user's Node version
require('@babel/preset-env').default,
{
targets: {
node: 'current',
},
},

However it looks like the use of modules: false for dependencies was intentional:
https://github.com/facebook/create-react-app/pull/3776/files#r161381985

@Timer

This comment has been minimized.

Copy link
Collaborator

commented Oct 2, 2018

Yeah, this was very intentional to prevent the ecosystem from publishing non-standard/supported language features to npm. We won't be adding an escape hatch for this.

For now, the other option to get small builds would be to install the single-function Lodash packages.

@jdalton

This comment has been minimized.

Copy link

commented Oct 2, 2018

@Timer

Hm, this is unfortunate that lodash ships separate packages for ESM and CommonJS. They should be under the same package.

Lodash shipped lodash-es, and lodash-amd, before the whole jsnext and module fields were a thing. It was shipping before Babel was a thing 🙃

The primary lodash package is already stuffed pretty full, clocking in at a bit over 4MB, because it has core builds, monolithic builds, minified builds, fp modules, etc. Being that lodash is the most depended upon package impacting over 162,495 packages its package size impacts users, so I don't want to add any more to it. The plan for the v5 release is to cut all the fat to ship a < 100kB primary package written in ESM and powered by the esm loader.

I see dual packages as a bit of an anti-pattern. They inflate your production package for a possible build/bundle-time optimization and have presented problems for the emerging, but still very much WIP, Node --experimental-modules when mixing CJS and ESM. I've been working on the esm loader, for the last 1.5 years, so folks can ship ESM-only package that work in all supported Node versions (helping folks transition from CJS to ESM).

For now, the other option to get small builds would be to install the single-function Lodash packages.

The single-function Lodash packages are soft-deprecated. It turns out trying to manage 400+ packages doesn't work out so well. They also lead to bigger bundles than simply using lodash or lodash-es and cherry-picking the modules from them.

@nerfologist

How to run jest when using es6 packages in node_modules

Jest supports specifying source transform and transformIgnorePatterns in its config files and defaults to using the babel-jest transform. In its next release esm will support being used as a Jest transform so you could use that was well. Something like:

{
  "transform": {
    "\\.m?js$": "esm"
  },
  "transformIgnorePatterns": []
}
@Timer

This comment has been minimized.

Copy link
Collaborator

commented Oct 2, 2018

All of those points are totally understood, I was just speaking in regards to what we typically see packages following.
I understand the uniqueness of lodash's position, such that it's a massive dependency and one of the most depended upon, predating these practices.

The plan for the v5 release is to cut all the fat to ship a < 100kB primary package written in ESM and powered by the esm loader.

It sounds like we'd probably just hold out for this to happen. Will the main key point to an esm-loader powered file and the module key point directly to ESM source?

Thanks for the background, @jdalton!

@jdalton

This comment has been minimized.

Copy link

commented Oct 2, 2018

@Timer

It sounds like we'd probably just hold out for this to happen. Will the main key point to an esm-loader powered file and the module key point directly to ESM source?

Yes! The main field will point to an index.js file which will use the esm-loader and the module field will point to the main.js which will be the ESM source entry.

@jangerhard

This comment has been minimized.

Copy link

commented Oct 4, 2018

I have the same issue with "react-redux".

 • Test suite failed to run

    Jest encountered an unexpected token

    This usually means that you are trying to import a file which Jest cannot parse, e.g. it's not plain JavaScript.

    [...]

    Details:

    ***\node_modules\react-redux\es\connect\connect.js:5
    import connectAdvanced from '../components/connectAdvanced';
    ^^^^^^

    SyntaxError: Unexpected token import

      4 | import LoginMethodComponent from './LoginMethodComponent';
      5 | import { startPollUpdateBankIDStatus } from '../../../redux/modules/bankID/userBankIDReducer';
    > 6 | import connect from 'react-redux/es/connect/connect';
        | ^
      7 | import { bindActionCreators } from 'redux';
      8 |
      9 | export class LoginMethodContainer extends React.Component {

      at ScriptTransformer._transformAndBuildScript (node_modules/jest-runtime/build/script_transformer.js:403:17)
      at Object.<anonymous> (src/components/user/login/LoginMethodContainer.js:6:1)
      at Object.<anonymous> (src/components/user/login/__tests__/LoginMethodContainer.test.js:3:1)

The temporary fix for now seems to be to specify "transformIgnorePatterns" as mentioned above:

"scripts": {
    ...
    "test": "react-scripts test --transformIgnorePatterns \"node_modules/(?!(react-redux))/\"",
    ...
  },
@stale

This comment has been minimized.

Copy link

commented Nov 3, 2018

This issue has been automatically marked as stale because it has not had any recent activity. It will be closed in 7 days if no further activity occurs.

@stale stale bot added the stale label Nov 3, 2018

@stale

This comment has been minimized.

Copy link

commented Nov 8, 2018

This issue has been automatically closed because it has not had any recent activity. If you have a question or comment, please open a new issue.

@stale stale bot closed this Nov 8, 2018

@Mr-Squirrel

This comment has been minimized.

Copy link

commented Nov 14, 2018

with mocha you simply require esm, probs same with jest

mocha -r esm

lodash/lodash#3837

sqs added a commit to sourcegraph/sourcegraph that referenced this issue Dec 15, 2018

remove lodash-es, just use lodash
lodash-es causes problems with jest (see facebook/create-react-app#5241 (comment)) and it is unnecessary for us to use it. We historically used it because it could be tree-shaken better than lodash, but now that we are using the babel lodash plugin, we get the same tree-shaking when we use plain lodash.

sqs added a commit to sourcegraph/sourcegraph that referenced this issue Dec 15, 2018

remove lodash-es, just use lodash
lodash-es causes problems with jest (see facebook/create-react-app#5241 (comment)) and it is unnecessary for us to use it. We historically used it because it could be tree-shaken better than lodash, but now that we are using the babel lodash plugin, we get the same tree-shaking when we use plain lodash.

sqs added a commit to sourcegraph/sourcegraph that referenced this issue Dec 15, 2018

remove lodash-es, just use lodash
lodash-es causes problems with jest (see facebook/create-react-app#5241 (comment)) and it is unnecessary for us to use it. We historically used it because it could be tree-shaken better than lodash, but now that we are using the babel lodash plugin, we get the same tree-shaking when we use plain lodash.

sqs added a commit to sourcegraph/sourcegraph that referenced this issue Dec 16, 2018

clean up usage of lodash (#1454)
* remove lodash-es, just use lodash

lodash-es causes problems with jest (see facebook/create-react-app#5241 (comment)) and it is unnecessary for us to use it. We historically used it because it could be tree-shaken better than lodash, but now that we are using the babel lodash plugin, we get the same tree-shaking when we use plain lodash.

* remove isEqual, flatten, compact helpers copied from lodash

Bundle size and bundling simplicity is not as big of a concern since we switched to stubbing `require('sourcegraph')` in Sep 2018. Now, these functions are only bundled with each extension if each extension itself uses them.

@lock lock bot locked and limited conversation to collaborators Jan 9, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.