-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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-config): parse testEnvironmentOptions
if received from CLI
#11902
fix(jest-config): parse testEnvironmentOptions
if received from CLI
#11902
Conversation
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!
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.
Wait, this seems like a bug - it should be parsed (the example in #11900 isn't JSON (it's JS literal) so that would fail, but that's separate).
https://github.com/facebook/jest/blob/8024306c365cdf8b17b2256c73f1f4c9f23f8f77/packages/jest-config/src/Defaults.ts#L67 is an object (and we use it as such internally: https://github.com/facebook/jest/blob/8024306c365cdf8b17b2256c73f1f4c9f23f8f77/packages/jest-environment-jsdom/src/index.ts#L37
I do think an improved description saying JSON string
is a good idea, but it should be parsed into an object before being passed to custom envs
Ah.. That’s right. I can fix this and also will add it to CLI docs for completeness. |
Awesome, thank you @mrazauskas! |
Codecov Report
@@ Coverage Diff @@
## main #11902 +/- ##
==========================================
- Coverage 68.73% 68.72% -0.01%
==========================================
Files 322 322
Lines 16582 16583 +1
Branches 4784 4785 +1
==========================================
Hits 11397 11397
- Misses 5152 5153 +1
Partials 33 33
Continue to review full report at Codecov.
|
testEnvironmentOptions
optiontestEnvironmentOptions
if received from CLI
@SimenB Perhaps it is not worth to include this option in the versioned CLI docs, because most probably the implementation is buggy. Or? |
Agreed 👍 You can add it do 27.2 as I'll publish a patch with this 🙂 |
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! just the one versioned docs change and this should be good to go 🙂
case 'transform': | ||
case 'haste': | ||
const str = argv[key]; | ||
if (isJSONString(str)) { |
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 wonder if this should throw if it returns false
... not for this PR anyways
…mrazauskas/jest into docs-fix-testEnvironmentOptions
Thanks! |
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. |
Resolves #11900
Summary
See #11900 (comment).
Test plan
I have added a unit test.