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 doesn’t respect Node resolution when looking for the CLI #5294

Closed
gaearon opened this issue Jan 12, 2018 · 12 comments · Fixed by #5304
Closed

Jest doesn’t respect Node resolution when looking for the CLI #5294

gaearon opened this issue Jan 12, 2018 · 12 comments · Fixed by #5304

Comments

@gaearon
Copy link
Contributor

gaearon commented Jan 12, 2018

This is related to #5119 (comment).

Our react-scripts package from Create React App depends on jest and runs it using the Node API. So jest is generally a dependency.

However, people often install top-level jest in their project (mostly because they don’t know it won’t work), and this breaks their setup with obscure errors like #5119. Their tree looks something like:

Ideally, top-level jest installations shouldn't break the copy of jest in react-scripts. This is a general assumption that typically holds true in the Node ecosystem: different versions of the same subdependency can coexist in different parts of the tree, and not break each other as long as you don’t mix their state.

In the case of Jest, it breaks because Jest uses <process.cwd()>/node_modules/jest-cli as the location of the CLI. Since in our project, process.cwd() is its root, Jest 22 “finds” jest-cli from Jest 20 and mistakingly uses it instead of its own copy of jest-cli:

├── [email protected] 
└─┬ [email protected] // <-- this is the calling code
  └── [email protected]             

├── [email protected] 
└─┬ [email protected] 
  └── [email protected] // <-- it calls Node API of this copy 

├── [email protected] // <-- but the inner copy "finds" the CLI from this copy and breaks
└─┬ [email protected] 
  └── [email protected]

I don’t know if this is intentional. Maybe this has to do with multi-project runner? Regardless, it would be nice if Jest either supported this, or failed with a clear message when Node API of one version of Jest resolves to the CLI code of another version of Jest.

@SimenB
Copy link
Member

SimenB commented Jan 13, 2018

This was introduced back in 39356eb

@gaearon
Copy link
Contributor Author

gaearon commented Jan 13, 2018

Wow, history. I think this makes sense if the launched CLI is global (outside of CWD) but the found CLI is local. However it doesn't make sense to me if both of them are inside of CWD.

@SimenB
Copy link
Member

SimenB commented Jan 13, 2018

I wonder if using https://www.npmjs.com/package/import-local would fix it.

@SimenB
Copy link
Member

SimenB commented Jan 14, 2018

See #5304.

cpojer pushed a commit that referenced this issue Jan 14, 2018
#5304)

* fix(jest-cli): use `import-local` to support global Jest installations

Fixes #5294

* docs: update changelog
@SimenB
Copy link
Member

SimenB commented Jan 15, 2018

We have the same issue with babel-jest, FWIW. See (esp. line 119): https://github.com/facebook/jest/blob/3ffc99e02d4b1ecad9d0f5ec2e228bddc8e7210b/packages/jest-config/src/normalize.js#L96-L125

If people install a babel-jest manually, we require that instead of our own. This is the reason for e.g. #5295 (comment).

Should we fix this?

@Merovex
Copy link

Merovex commented Mar 14, 2018

I just started a new CRA and ran into the missing jest-cli issue and "TypeError: environment.setup is not a function" (also noob to React)

  "dependencies": {
    "bootstrap": "^4.0.0",
    "jest-cli": "^22.4.2",
    "jquery": "^3.3.1",
    "node-sass-chokidar": "^1.1.0",
    "npm-run-all": "^4.1.2",
    "popper.js": "^1.14.1",
    "react": "^16.2.0",
    "react-dom": "^16.2.0",
    "react-router": "^4.2.0",
    "react-router-dom": "^4.2.2",
    "react-scripts": "1.1.1",
    "react-transition-group": "^2.2.1",
    "reactstrap": "^4.8.0"
  },

Turns out, I had to remove --env=jsdom from my script:

"scripts": {
    "start": "npm-run-all -p watch-css start-js",
    "start-js": "react-scripts start",
    "build": "npm-run-all build-css build-js",
    "build-js": "react-scripts build",
    "test": "react-scripts test",
    "NOeject": "react-scripts eject",
    "test:debug": "react-scripts --inspect-brk test --runInBand --env=jsdom",
    "jest-enzyme": "^6.0.0"
  },

@SimenB
Copy link
Member

SimenB commented Mar 14, 2018

CRA comes with jest built in. You need to remove it from your own package.json. See facebook/create-react-app#1795 and the many linked discussions within

@Merovex
Copy link

Merovex commented Mar 14, 2018

Yeah, that's not the optimal answer. I read facebook/create-react-app#1795, and the many linked discussions. Turns out that --env=jsdom was throwing the "TypeError: environment.setup" error. I removed it and got to bliss.

This is a virgin CRA app, nothing of note added but the packages. Following the advice of "removing Jest," which wasn't in my packages at any rate...

"dependencies": {
    "bootstrap": "^4.0.0",
    "jquery": "^3.3.1",
    "node-sass-chokidar": "^1.1.0",
    "npm-run-all": "^4.1.2",
    "popper.js": "^1.14.1",
    "react": "^16.2.0",
    "react-dom": "^16.2.0",
    "react-router": "^4.2.0",
    "react-router-dom": "^4.2.2",
    "react-scripts": "1.1.1",
    "react-transition-group": "^2.2.1",
    "reactstrap": "^4.8.0"
  },

Yields:

> react-scripts test

module.js:559
    throw err;
    ^

Error: Cannot find module '/Users/.../node_modules/jest-cli'
    at Function.Module._resolveFilename (module.js:557:15)
    at Function.Module._load (module.js:484:25)
    at Module.require (module.js:606:17)
    at require (internal/module.js:11:18)
   <...etc...?
npm ERR! Test failed.  See above for more details.

I install a few packages to minimally get past errors resulting in:

  "dependencies": {
    "bootstrap": "^4.0.0",
    "enzyme": "^3.3.0",
    "enzyme-adapter-react-16": "^1.1.1",
    "jest-cli": "^22.4.2",
    "jest-enzyme": "^6.0.0",
    "jquery": "^3.3.1",
    "node-sass-chokidar": "^1.1.0",
    "npm-run-all": "^4.1.2",
    "popper.js": "^1.14.1",
    "react": "^16.2.0",
    "react-dom": "^16.2.0",
    "react-router": "^4.2.0",
    "react-router-dom": "^4.2.2",
    "react-scripts": "1.1.1",
    "react-transition-group": "^2.2.1",
    "reactstrap": "^4.8.0"
  },

and a test that should properly fail

 FAIL  src/App.test.js
  ✓ renders without crashing (3ms)
  ✕ renders welcome message (13ms)

  ● renders welcome message

    expect(received).toEqual(expected)
    
    Expected value to equal:
      true
    Received:
      false

      29 |   const welcome = <h2>Welcome to React</h2>;
      30 |   // expect(wrapper.contains(welcome)).to.equal(true);
    > 31 |   expect(wrapper.contains(welcome)).toEqual(true);
      32 | });
      33 | 
      
      at Object.<anonymous>.it (src/App.test.js:31:37)

@SimenB
Copy link
Member

SimenB commented Mar 15, 2018

You can't have jest-cli in your own package.json when using CRA. You should remove it, as well as any lockfile, and run install again

@gaearon
Copy link
Contributor Author

gaearon commented Apr 3, 2018

To folks commenting here: Jest tracker isn't the best place to report issues about CRA :-) This way we don't hear about them and people spend weeks with broken setups while we're completely unaware. Please file an issue in CRA itself.

@gaearon
Copy link
Contributor Author

gaearon commented Apr 3, 2018

I published [email protected] which should fix environment.dispose is not a function for you.

https://github.com/facebook/create-react-app/releases/tag/v1.1.3

@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.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants