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

fix: jest-util should not depend on jest-mock #4992

Merged
merged 2 commits into from
Nov 30, 2017

Conversation

SimenB
Copy link
Member

@SimenB SimenB commented Nov 30, 2017

Summary
jest-util should not depend on jest-mock.

Test plan
Green CI

Copy link
Collaborator

@thymikee thymikee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're going to get rid of that module anyway, right? But until then, 👍

@SimenB
Copy link
Member Author

SimenB commented Nov 30, 2017

Should this be done as well?

diff --git i/packages/jest-config/package.json w/packages/jest-config/package.json
index 2094d0ff..252b8046 100644
--- i/packages/jest-config/package.json
+++ w/packages/jest-config/package.json
@@ -10,8 +10,6 @@
   "dependencies": {
     "chalk": "^2.0.1",
     "glob": "^7.1.1",
-    "jest-environment-jsdom": "^21.2.1",
-    "jest-environment-node": "^21.2.1",
     "jest-get-type": "^21.2.0",
     "jest-jasmine2": "^21.2.1",
     "jest-regex-util": "^21.2.0",
@@ -19,5 +17,9 @@
     "jest-util": "^21.2.1",
     "jest-validate": "^21.2.1",
     "pretty-format": "^21.2.1"
+  },
+  "devDependencies": {
+    "jest-environment-jsdom": "^21.2.1",
+    "jest-environment-node": "^21.2.1"
   }
 }

jest-config default to jsdom as env, and does require.resolve(`jest-environment-${env}`); , but no actual usage of the modules

@SimenB SimenB force-pushed the jesy-util-depend-on-jest-mock branch from 71394a1 to 22d9da0 Compare November 30, 2017 19:33
"jest-validate": "^21.2.1",
"mkdirp": "^0.5.1"
},
"devDependencies": {
"jest-mock": "^21.2.0"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's imported directly in the tests

@cpojer cpojer merged commit 4e2f41f into jestjs:master Nov 30, 2017
@thymikee
Copy link
Collaborator

Hm, it's funny, because env is not a direct dependency for jest-config, but jsdom and node serve as default ones. With flat dependency tree (which is the only one we support now) it doesn't really matter, but I'd leave it as deps anyway. If moving anywhere (which I'm not a fan), it would rather be optionalDependencies instead of devDependencies, no?

@cpojer
Copy link
Member

cpojer commented Nov 30, 2017

Yeah, Jest should come with the node and jsdom environments by default, even if only one of them is used.

@SimenB SimenB deleted the jesy-util-depend-on-jest-mock branch November 30, 2017 19:40
@SimenB
Copy link
Member Author

SimenB commented Nov 30, 2017

Jest should come with the node and jsdom environments by default

This is jest-config, though. jest-runtime has a dependency on it already, so the deps will still get there by default

mjesun added a commit that referenced this pull request Dec 4, 2017
mjesun added a commit that referenced this pull request Dec 4, 2017
cpojer pushed a commit that referenced this pull request Dec 4, 2017
* Revert "docs: update expect.anything() example (#5007)"

This reverts commit ea3fabc.

* Revert "Emphasise required return (#4999)"

This reverts commit 4f1113c.

* Revert "Makes NPM a link like Yarn is (#4998)"

This reverts commit aa4f09d.

* Revert "Update Timeout error message to `jest.timeout` and display current timeout value (#4990)"

This reverts commit 08f8394.

* Revert "fix: jest-util should not depend on jest-mock (#4992)"

This reverts commit 4e2f41f.

* Revert "Update Troubleshooting.md (#4988)"

This reverts commit 6414f28.

* Revert "Revert "Add the Yammer logo to the 'who is using this' section of the website." (#4987)"

This reverts commit 91b104f.

* Revert "Make "weak" optional dependency and check it at runtime (#4984)"

This reverts commit e00529d.

* Revert "Re-inject native Node modules (#4970)"

This reverts commit ef55e89.
@wuno
Copy link

wuno commented Dec 29, 2017

Would someone please explain the fix for this? I was not clear after reading through this problem.

I setup my app with

Create React App

When I run npm test I get errors.

First it was jest-cli that I needed. Once I installed jest-cli I started seeing this error,

TypeError: environment.setup is not a function

When I run,

npm ls jest-environment-node

├─┬ [email protected]
│ │ └── [email protected]
│ └─┬ [email protected]
│   └─┬ [email protected]
│     └── [email protected]
└─┬ [email protected]
  └─┬ [email protected]
    └─┬ [email protected]
      └─┬ [email protected]
        └── [email protected]

This is my .babelrc

{
  "plugins": [
    "transform-flow-strip-types",
  ],
  "presets": [
    "env",
    "react"
  ]
}

This is my package.json


  "scripts": {
    "start": "react-scripts start",
    "flow": "flow",
    "lint": "eslint .",
    "prettier": "find . -name node_modules -prune -or -name '*.js' -print | xargs prettier --write",
    "build": "react-scripts build",
    "test": "react-scripts test --env=jsdom",
    "eject": "react-scripts eject"
  },
  "dependencies": {
    "bootstrap": "^3.3.7",
    "lodash": "^4.17.4",
    "react": "^16.1.0",
    "react-bootstrap": "^0.31.5",
    "react-dom": "^16.1.0",
    "react-fontawesome": "^1.6.1",
    "react-google-maps": "^9.4.3",
    "react-redux": "^5.0.6",
    "react-router-bootstrap": "^0.24.4",
    "react-router-dom": "^4.2.2",
    "react-scripts": "1.0.17",
    "react-tabs": "^2.1.1",
    "recharts": "^1.0.0-beta.7",
    "redux": "^3.7.2"
  },
  "devDependencies": {
    "babel-cli": "^6.26.0",
    "babel-eslint": "^8.0.2",
    "babel-plugin-transform-flow-strip-types": "^6.22.0",
    "babel-preset-env": "^1.6.1",
    "babel-preset-react": "^6.24.1",
    "eslint": "^4.11.0",
    "eslint-config-fb-strict": "^21.2.0",
    "eslint-config-fbjs": "^2.0.0",
    "eslint-plugin-babel": "^4.1.2",
    "eslint-plugin-flowtype": "^2.39.1",
    "eslint-plugin-import": "^2.8.0",
    "eslint-plugin-jsx-a11y": "^6.0.2",
    "eslint-plugin-prettier": "^2.3.1",
    "eslint-plugin-react": "^7.4.0",
    "flow-bin": "^0.59.0",
    "jest-cli": "^22.0.4",
    "prettier": "^1.8.2",
    "redux-devtools": "^3.4.1"
  }
}

@SimenB
Copy link
Member Author

SimenB commented Dec 30, 2017

No need to post in multiple threads 🙂 See answer in the other one

@github-actions
Copy link

This pull request 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 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants