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

Delete or allow changing Jest transformIgnorePatterns option so tests won't fail when importing ES2015 modules from node_modules #2943

Closed
gykf opened this issue Aug 11, 2017 · 6 comments

Comments

@gykf
Copy link

gykf commented Aug 11, 2017

Is this a bug report?

No, it's not.

This is a proposal to delete the Jest configuration option transformIgnorePatterns in package.json of new CRA apps, or at least enable changing the option without ejecting.

Users of CRA may want to import 3rd-party modules written in ES2015 from the node_modules directory. This works as expected when using the Webpack build. However, it fails when running Jest tests on the module which imports an ES2015 module from node_modules. The test runner throws a SyntaxError (see #2940) because Jest doesn't transform the dependencies in node_modules.

I don't think it should be a requirement to pre-compile modules in node_modules to an old version of JS before running Jest tests with them. Users may not have control over the modules in node_modules, being unable to pre-compile them before running the tests (without forking them). Importing ES2015 modules from node_modules works as expected when using Webpack, there's no requirement to pre-compile them in a separate step before running the Webpack build. Files and modules that get compiled without issue with Webpack fail to run with Jest. This behaviour between Webpack and Jest is inconsistent, thus unexpected and confusing for me and probably other users as well.

So I suggest removing the transformIgnorePatterns strings from package.json. If there are any concerns regarding the transformation of modules in node_modules (e.g. speed) with Jest, I would at least want to have a way of changing the option without ejecting.

@Timer
Copy link
Contributor

Timer commented Aug 11, 2017

Sorry, but allowing configuration of this is not the solution to resolve improperly published modules.

We should focus on educating or providing a tool for the community to publish modules which require no compilation before being consumed.

@Timer Timer closed this as completed Aug 11, 2017
@gykf
Copy link
Author

gykf commented Aug 11, 2017

@Timer I don't see why publishing packages using valid ES2015 syntax is 'improper'. ES2015 modules can be used in Safari 10.1 and Chrome 61 beta without a compilation step, they 'require no compilation before being consumed'. BTW, I don't see an explanation for why the discrepancy between how Webpack and Jest consumes ES2015 modules is advantageous. Why provide a solution to consume valid ES2015 files with Webpack, but not with Jest?

The code under test is ultimately supposed run in the browser. However, presumably to speed up testing, Jest runs them using JSDOM, a library meant to be used in a Node.js environment. This choice to run tests in Node as opposed to a browser was made by Jest developers, much like the choice to use Jest in CRA. The lack of support for syntactically valid, standard ES2015 modules (for 2 years now) is a deficiency of Node.js. The lack of support for CommonJS modules in packages that were never even meant to be run in Node.js is not the issue here. But Jest can already provide a compilation step to make up for this deficiency in Node.js. I wish to use this feature without having to eject my CRA project. That is all.

@Timer
Copy link
Contributor

Timer commented Aug 11, 2017

Packages should always be pre-compiled to their lowest target environment.
In CRA apps, we enforce an IE 9+ compatibility requirement.

Compiling dependencies is slow and unreliable; code should be shipped in both common js and esmodules, or only common js.
There are fields in package.json specifying the differing builds.

@Timer Timer reopened this Aug 11, 2017
@Timer Timer closed this as completed Aug 11, 2017
@Timer
Copy link
Contributor

Timer commented Aug 11, 2017

Please open an issue with the dependency you're trying to use and ask them to fix their publish.

@gykf
Copy link
Author

gykf commented Aug 11, 2017

@Timer OK, I get that package authors can define different fields for CJS and ES modules in package.json (which is only a draft proposal, not a standard, and it's not documented). (Edit: Some packages are only supposed to run in the browser, not in Node.js, and CRA is a tool to create browser apps. Webpack takes care of ensuring that the output is compatible with IE9+.) I consider this fact irrelevant to our discussion.

Again, the inconsistent behaviour of Webpack and Jest is still not addressed. Why require users/package authors to pre-compile ES modules when using Jest? Or, rather, why not require users/package authors to always pre-compile ES modules before using them with Webpack?

CRA is a tool to create apps that run in the browser, and Jest is a tool to test these web apps that run in the browser. Modern browsers can interpret ES modules perfectly fine (browsers can't do anything with package.json files, so they're irrelevant). My interpretation of "browser support" meant that CRA users should be able to use Jest to test any code that runs in supported browsers (including ES modules). But it looks like "browser support" just means that, whatever the output of Webpack is, it will run in supported browsers. Code that runs fine without any pre-processing in browsers may not be supported by CRA. So ES2015 support in CRA, a tool to create browser apps, is not on par with some of the browsers on the market today, used by ordinary people already, and will be behind as long as Node doesn't support ES modules (which will probably take years, last time I checked). Seems ironic to me, when my expectation toward CRA was that I could leverage modern ES language features in older browsers today.

@corlaez
Copy link

corlaez commented Jan 17, 2018

Dan seems to car though: #2537

@lock lock bot locked and limited conversation to collaborators Jan 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants