-
Notifications
You must be signed in to change notification settings - Fork 31
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
chore: update peerdependency of jest to ^25.5.4 @W-7689277 #90
Conversation
We should probably also make babel-jest-preset a peer dependency as well since this change will require a major version bump. |
@@ -30,7 +30,7 @@ | |||
"@lwc/jest-resolver": "7.0.0", | |||
"@lwc/jest-serializer": "7.0.0", | |||
"@lwc/jest-transformer": "7.0.0", | |||
"jest-environment-jsdom-fifteen": "^1.0.2" | |||
"jest-environment-jsdom-fifteen": "~1.0.2" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only change is to use the right semver qualifier for dependency. The version in yarn.lock remains the same.
265127a
to
f5ef149
Compare
f5ef149
to
b38fd4e
Compare
}, | ||
"devDependencies": { | ||
"@lwc/jest-preset": "7.0.0", | ||
"@lwc/jest-resolver": "7.0.0", | ||
"@lwc/jest-serializer": "7.0.0" | ||
"@lwc/jest-serializer": "7.0.0", | ||
"babel-preset-jest": "^25.5.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding this as a devDependency here as the unit tests in @lwc/jest-transformer rely on this dependency.
@@ -30,7 +30,7 @@ | |||
"@lwc/jest-resolver": "7.0.0", | |||
"@lwc/jest-serializer": "7.0.0", | |||
"@lwc/jest-transformer": "7.0.0", | |||
"jest-environment-jsdom-fifteen": "^1.0.2" | |||
"jest-environment-jsdom-fifteen": "~1.0.2" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
jest-environment-jsdom-fifteen
is not needed anymore since jest
25 depends on jsdom 15: https://github.com/facebook/jest/blob/v25.5.4/packages/jest-environment-jsdom/package.json#L25
}, | ||
"peerDependencies": { | ||
"@lwc/compiler": "*", | ||
"jest": ">= 24.9.0" | ||
"babel-preset-jest": "^25.5.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not certain if babel-preset-jest
should be peer dependency. Since babel-preset-jest
is already pulled down by jest
, and by using a peer dependency we might run into a version mismatch. What do you think about resolving the babel-preset-jest
from the jest
module?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When we tested it, the babel-preset-jest
dependency was in the node_modules of jest-config
rather than at the top level. We could dig around node_modules to find it. That feels a bit fragile though, jest-config
doesn't even have a README.
Or maybe there's a more reliable way to get the babel-preset-jest
from jest
?
554f319
to
4027824
Compare
4027824
to
4ed55d3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One more thing, please add jest-resolve
as a dependency of @lwc/jest-resolver
.
|
||
// Use an up-to-date version of jsdom. Jest v24 comes with jsdom v11 which doesn't offer support | ||
// for native shadow DOM. | ||
testEnvironment: 'jest-environment-jsdom-fifteen', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also remove jest-environment-jsdom-fifteen
from:
- base.config.js
- @lwc/jest-preset/package.json
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They have been removed in the same commit 4ed55d3 as this change.
@@ -24,13 +24,12 @@ | |||
"peerDependencies": { | |||
"@lwc/compiler": "*", | |||
"@lwc/synthetic-shadow": "*", | |||
"jest": ">=24.9.0" | |||
"jest": "^25.5.4" |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
No description provided.