-
-
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: update yargs to fix CLI flag overriding #9519
Conversation
Codecov Report
@@ Coverage Diff @@
## master #9519 +/- ##
======================================
Coverage 65% 65%
======================================
Files 283 283
Lines 12151 12151
Branches 3010 3010
======================================
Hits 7899 7899
Misses 3608 3608
Partials 644 644 Continue to review full report at Codecov.
|
I never knew overriding flags like this was supported. Before rolling back, I'd rather like to consult with Yargs team if this is expected behavior or a bug. Can you forward mentioned issue to them? |
I've pinged Ben privately |
I could add introducing |
@thymikee regardless, this is either a bug or an oversight in our config normalization - we pass config from file through @jeysal if not a big effort I'd welcome it 😀 |
That just checks for deprecated entries and typos, it doesn't validate the shape of the object's values. An alternative is to merge diff --git i/packages/jest-config/src/normalize.ts w/packages/jest-config/src/normalize.ts
index a2454c373..4e86339c6 100644
--- i/packages/jest-config/src/normalize.ts
+++ w/packages/jest-config/src/normalize.ts
@@ -457,7 +457,9 @@ export default function normalize(
hasDeprecationWarnings: boolean;
options: AllOptions;
} {
- const {hasDeprecationWarnings} = validate(initialOptions, {
+ const argvAndInitialOptions = setFromArgv(initialOptions, argv);
+
+ const {hasDeprecationWarnings} = validate(argvAndInitialOptions, {
comment: DOCUMENTATION_NOTE,
deprecatedConfig: DEPRECATED_CONFIG,
exampleConfig: VALID_CONFIG,
@@ -476,7 +478,7 @@ export default function normalize(
let options = normalizePreprocessor(
normalizeReporters(
normalizeMissingOptions(
- normalizeRootDir(setFromArgv(initialOptions, argv)),
+ normalizeRootDir(argvAndInitialOptions),
configPath,
projectIndex,
), That explodes for the different options we have that's CLI-only though, as those are included in the |
That somehow broke coverage reports? Seems like it collapsed |
Will also be fixed by yargs@16: yargs/yargs#1556 |
3361486
to
3584351
Compare
This comment has been minimized.
This comment has been minimized.
Codecov Report
@@ Coverage Diff @@
## master #9519 +/- ##
=======================================
Coverage 65.09% 65.09%
=======================================
Files 287 287
Lines 12145 12145
Branches 3007 3007
=======================================
Hits 7906 7906
Misses 3604 3604
Partials 635 635 Continue to review full report at Codecov.
|
fix: update yargs to fix CLI flag overriding (jestjs#9519)
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. |
Summary
Fixes #9517. @jeysal we really need to add
io-ts
or something to our config as we've talked about multiple times 😬Test plan
Test added. Without rolling back yargs it fails.