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

Jest 23 --config breaks create-react-app on Windows #6385

Closed
bugzpodder opened this issue Jun 3, 2018 · 19 comments · Fixed by #6523
Closed

Jest 23 --config breaks create-react-app on Windows #6385

bugzpodder opened this issue Jun 3, 2018 · 19 comments · Fixed by #6523

Comments

@bugzpodder
Copy link

bugzpodder commented Jun 3, 2018

I am trying to use Jest 23 in create-react-app@next facebook/create-react-app#4555
In particular I've found windows config to break after the upgrade.
The specific part of config being generated is https://github.com/facebook/create-react-app/blob/next/packages/react-scripts/scripts/utils/createJestConfig.js#L47
In Jest 22.4.3 the regex is [\\\\\\\\\\\\]node_modules[\\\\\\\\\\\\].+\.(js|jsx|mjs)$|^.+\.module\.(css|sass|scss)$/ (12 slashes) in jest-runtime
In Jest 23 the error is Invalid regular expression: /[\\\\\]node_modules[\\\\\].+\.(js|jsx|mjs)$|^.+\.module\.(css|sass|scss)$/

Does not repo on other platforms.

@bugzpodder bugzpodder changed the title Upgrading Jest to 23 breaks create-react-app config on Windows Jest 23 --config breaks create-react-app on Windows Jun 3, 2018
@bugzpodder
Copy link
Author

bugzpodder commented Jun 3, 2018

repo case:
different behaviors on Jest 23/22.4.3 on windows:
jest --config={\"transformIgnorePatterns\":[\"[/\\]node_modules[/\\]\"]}
#5941 #2381

I don't know the new behavior is intended (ie the way we were doing transformIgnorePatterns was wrong) or if its a jest 23 regression.
It's not an issue on linux.

@SimenB
Copy link
Member

SimenB commented Jun 4, 2018

Only regex change I can find in the changelog is #5941. Does reverting that fix it for you?

It'd be awesome if you could put together a unit (or integration) test that passes against jest 22, but fails for jest 23

@bugzpodder
Copy link
Author

Yep #5941 is the one that affected me. I can look into putting down a unittest after you guys figure out what the correct resolution is (see my previous comment w/ a small repo case).

@SimenB
Copy link
Member

SimenB commented Jun 4, 2018

None on the core team uses windows, so we'll have to rely on the community to fix this. But having a test in the repo (skipped on windows) would be great for a potential contributor to jump in :)

@TroySchmidt
Copy link

@bugzpodder This is breaking my WallabyJS test running on Windows. Is there something you did to the configuration to override the effects of #5941 on breaking Jest 23?

@bugzpodder
Copy link
Author

bugzpodder commented Jun 5, 2018

@TroySchmidt I basically changed:
jest --config={\"transformIgnorePatterns\":[\"[/\\]node_modules[/\\]\"]}
to
jest --config={\"transformIgnorePatterns\":[\"/node_modules/\"]}
I don't know if this is the correct change, hopefully this helps you.

@tshinnic
Copy link

tshinnic commented Jun 6, 2018

Speculating...

jest-config/build/index.js::readConfig() calls normalize() on incoming option values and defaults, including transformIgnorePatterns.

When normalize() sees
options.transformIgnorePatterns: "[/\\]node_modules[/\\].+\.(js|jsx|mjs)$"
it converts that to
value: "[\\\\\]node_modules[\\\\\].+\.(js|jsx|mjs)$"

Note that really is 5 back slashes, 2 each from each incoming back slash and 1 from the forward slash. An odd number of back slashes produces much unhappiness for a later new RegExp().

normalize() uses normalizeUnmockedModulePathPatterns() which uses 'jest-regex-util'. Checking the results returned by that module's code finds:

> "/\\\\".replace(/(\/|\\(?![[\]{}()*+?.^$|]))/g, '\\\\').split('')
          [ '\\', '\\', '\\', '\\', '\\', '\\' ]
> "[/\\\\]".replace(/(\/|\\(?![[\]{}()*+?.^$|]))/g, '\\\\').split('')
          [ '[', '\\', '\\', '\\', '\\', '\\', ']' ]

so path separators are okay unless they happen to have char class delimiters around them. Perhaps the wrong routine is being used? Or rather, that one routine is being called for incompatible uses.

Oh, the rules wrt to '[' and ']' got changed by edd95fc and #5941

You probably do want separate routines...

@SimenB
Copy link
Member

SimenB commented Jun 6, 2018

@hron thoughts?

@ThakurKarthik
Copy link

SyntaxError: Invalid regular expression: /[\\]node_modules[\\].+.(js|jsx|mjs)$/: Unterminated character class
at new RegExp ()

  at shouldTransform (node_modules/jest-runtime/build/script_transformer.js:603:9)

@ThakurKarthik
Copy link

what is the problem? My os is windows 10.
My package.json
{ "name": "create-react-app-antd", "version": "0.1.0", "private": true, "dependencies": { "antd": "^2.13.8", "prop-types": "^15.6.1", "react": "^16.0.0", "react-dom": "^16.0.0", "react-redux": "^5.0.7", "react-router-dom": "^4.3.1", "react-scripts": "1.0.16", "redux": "^4.0.0", "styled-components": "^3.3.2" }, "scripts": { "start": "react-app-rewired start", "build": "react-app-rewired build", "test": "react-app-rewired test --env=jsdom", "eject": "react-scripts eject" }, "devDependencies": { "babel-eslint": "^8.2.3", "eslint": "^4.19.1", "eslint-config-import": "^0.13.0", "eslint-plugin-jsx-a11y": "^6.0.3", "eslint-plugin-react": "^7.9.1", "jest-enzyme": "^6.0.1", "eslint-config-airbnb": "^16.1.0", "babel-plugin-import": "^1.6.2", "enzyme": "^3.3.0", "enzyme-adapter-react-16": "^1.1.1", "jest": "^23.1.0", "jest-cli": "^23.1.0", "jest-styled-components": "^5.0.1", "react-app-rewire-less": "^2.0.8", "react-app-rewire-styled-components": "^3.0.2", "react-app-rewired": "^1.5.2", "react-app-rewire-eslint": "^0.2.3", "react-test-renderer": "^16.4.0" } }

@bugzpodder
Copy link
Author

@ThakurKarthik what errors are you seeing? You should be running npm run test or yarn test

@tshinnic
Copy link

tshinnic commented Jun 16, 2018

@bugzpodder Programs generated with create-react-app are dying on Windows when/if jest is updated to 23 to fix other errors. This happens with a simple npm run test and with a specific file mentioned npm run test src/rdcrsSlugCharAssembly.test.js The error is as mentioned in @ThakurKarthik 's first comment

#5941 tried to fix original issue #2381 which had path members like "..../(breaks)/....", by changing a regex to try to avoid modifying an escape backslash when it was used to escape a regexp special characters.

This fix now trips up on path regexes such as used by create-react-app like the above repeatedly mentioned [/\\]node_modules[/\\].+\.(js|jsx|mjs)$ because regexp special characters are used more generally, within/inside path member strings.

Ahhh... got it!

Key here is that "[/\\]" is really [ '[', '/', '\\', '\\', ']' ] . That is, it has an escaped escape, not either an isolated escape char or an escape followed by another special char. And... the new regex is not checking for that.

The jest-regex-util line 26

    return string.replace(/(\/|\\(?![[\]{}()*+?.^$|]))/g, '\\\\');`

should be changed to add \\ inside the negative lookahead character set

    return string.replace(/(\/|\\(?![[\]{}()*+?.^$|\\]))/g, '\\\\');`

When I do that, the problem invalid regexp result (odd number of backslashes)
jest-config/build/normalize.js::normalize: value: "[\\\\\]node_modules[\\\\\].+\.(js|jsx|mjs)$"
is changed to the good result
jest-config/build/normalize.js::normalize: value: "[\\\\]node_modules[\\\\].+\.(js|jsx|mjs)$"
and with that one change Jest tests are running again for my create-react-app generated app.

More tests would be wonderful I'm sure and your being able to test on Windows would be even better. Consider testing this fix under your available platforms.

@SimenB
Copy link
Member

SimenB commented Jun 16, 2018

Would you be able to provide a PR? We have CI running on appveyor, so if we can land a fix with a test we should hopefully not regress again 🙂

@ThakurKarthik
Copy link

I have downgraded jest and jest-cli from v 23.1.0 to v 22.4.4.Now it's working fine.This error is only on windows platform.

peteruithoven added a commit to peteruithoven/jest that referenced this issue Jun 21, 2018
Should fix: jestjs#6385
Credits for fix go to @tshinnic
@hron
Copy link
Contributor

hron commented Jun 22, 2018

I've been thinking on this issue for quite a long time and it seems there is no easy way to fix it. Essentially this function should be very intelligent to decide if '\\' should be replaced because it's a path separator or it shouldn't if it's an escaping regex symbol (e.g. '\\]'). One of the possible solution might be to iterate over all possible replacements of the '\' and test if it's a valid regex, but it could lead to some unpredictable results.

Maybe it's better to avoid using '\\' as a path separators in '*Patterns' configuration variables at all? Jest can specify this in the documentation and we can transform / to '[/\\\\]' on Windows automatically.

@ThakurKarthik
Copy link

For now we have to use the downgraded version

@hron
Copy link
Contributor

hron commented Jun 22, 2018

After further consideration it seems the users uses '\\\\' to specify backslash in *Patterns. In that case the fix is possible.

@kwaazaar
Copy link

I added '--no-watchman' and it worked.

@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 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
7 participants