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.

Sign up for GitHub

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

Jump to bottom

fix(jest-transformer): add babel-preset-jest dependency #1112

Merged
merged 2 commits into from
Mar 18, 2019
Merged

fix(jest-transformer): add babel-preset-jest dependency #1112

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
07abf05
fix(jest-transformer): explicitly add babel-preset-jest dependency
Mar 14, 2019
2ac552b
wip: jest as peer dep to jest-transformer
Mar 18, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion packages/@lwc/jest-transformer/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,13 @@
"@babel/plugin-transform-modules-commonjs": "7.1.0",
"@babel/template": "~7.1.2",
"@lwc/errors": "0.37.0",
"babel-preset-jest": "^24.0.0",
"deasync": "0.1.14"
},
"peerDependencies": {
Copy link
Member

Choose a reason for hiding this comment

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

Shall we add instead jest as a peer dependency?
How does other jest transformers deal with this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can add jest as a peer dependency, but we still need a dependency here on babel-preset-jest. This package works with the latest versions of Jest (and I encourage users to stay relatively current), but babel-preset-jest isn't accessible from this package even if the consuming project has a jest dependency.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, I don't think we need jest as a peer dependency here. This package declares it's own dependencies for the what it uses and the input/output of the transformer hasn't changed in recent versions. It takes in the source code and file path and returns the compiled/transformed code. That should work for older and newer versions of Jest.

We can add a peer dependency for Jest 24 because that's what we want consumers to use but it won't break them to be on Jest 23 so I don't think we should burden them with the warning message if it's not a hard requirement.

Copy link
Member

Choose a reason for hiding this comment

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

You are assuming here that jest has not introduced a breaking change in babel-preset-jest between 23 and 24.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The only breaking change Jest listed between 23 and 24 for this package was for to a change that required Babel 7, but the transformer has been on 7 for a while.

I can add the peer dependency just to be safe. I feel like this is moving in a direction where we want the Jest related packages in here to match the Jest major versions though. Similar to how ts-jest does it.

"@lwc/compiler": "0.37.x",
"@lwc/engine": "0.37.x"
"@lwc/engine": "0.37.x",
"jest": "24.x"
},
"devDependencies": {
"@lwc/compiler": "0.37.0",
Expand Down