-
Notifications
You must be signed in to change notification settings - Fork 156
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
feat: update to support babelConfig and tsConfig #111
Conversation
361918d
to
afcd2ac
Compare
You may notice the configuration option is called is I decided to make the external interface align closer with the nomenclature used by other community tools, including babel itself. |
This looks great, I have some thoughts on the API. I think the {
"jest": {
"globals": {
"vue-jest": {
"babelOptions": {
"presets": [
[
"env",
{
"useBuiltIns": "entry",
"shippedProposals": true
}
]
],
"plugins": [
"syntax-dynamic-import"
],
"env": {
"test": {
"plugins": [
"dynamic-import-node"
]
}
}
}
}
}
}
} Or {
"jest": {
"globals": {
"vue-jest": {
"babelOptions": "relative/path/to/file"
}
}
}
} Also we need full tests to check that both API options work. Other than that, looks great :) |
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.
Please update the API to match the ts-jest API, and add tests (not just unit) to check that both API options work. Other than that, looks great :)
@eddyerburgh - To match the ts-jest API, it would result in a breaking change to the current default logic for From the docs:
Currently, even if no configuration is provided, the default fallback logic for So the question is,
Also, can you clarify what you mean by "not just unit tests"? It looks to me like that's all there is |
9555e0d
to
685eaf2
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.
Requested changed done, please see comments relating to final solution.
I rebased against master to get #108 which is why the "Review Changes" doesn't link to the original commit.
03aa01e
to
1a1d560
Compare
Yes
I think we should deprecate the option as part of this work. To deprecate it, we need to remove it from the docs and add a warning when the method is called (without removing functionality). I also think we should keep the API the same between Sorry for the amount of work, but you highlighted an issue with our globals design and I think we should address it now. Also, the next release will be a major due to this PR which changes the behavior of the babelrc lookup |
Sure, no worries. I was coming to the same conclusion. |
205f1ad
to
a8714cf
Compare
Requested changes in place. Build will fail, because there is a prerequisite on #113 |
a8714cf
to
fba1043
Compare
@eddyerburgh - rebased against #113; should be good to merge. |
fff7ba9
to
4daf24a
Compare
- babelConfig/tsConfig can take a boolean (config file lookup), object (inline config options), or string (path to config file) - deprecated babelRcFile and tsConfigFile options - Matches ts-jest API for babelConfig/tsConfig, with the exception that the default behavior for babelConfig is switched (enabled) - Not a breaking change
4daf24a
to
bb80fd9
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.
Good work :)
Hey @justinhelmer I decided that we should just let ts-jest and babel-jest handle compiling, like you suggested. Would you like to work on ts-jest integration? |
@eddyerburgh - I believe you are referring to #114 . Unfortunately I don't have the bandwidth right now to take on this challenge. It might change in the future if someone else doesn't take this on, but I don't want to be a bottleneck for delivery. |
Fixes #110