Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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: warn when there are multiple configs #11922
feat: warn when there are multiple configs #11922
Changes from 13 commits
b85b26b
4dff740
227f7d4
a678d28
0f81fe4
b6ff8b8
27fdaff
0d15a1f
eccf953
3d71585
4cca511
752b1f9
02b8ef2
cd7c9c9
9e13ff2
f30ab95
b931029
f9c515e
3b1c5b8
02338fd
2ce0401
ac00510
82d3e44
db8c90c
ab05377
df0f3a5
1f17acc
2d6efff
3682947
7727e74
9926edf
91655ce
d60356f
6a52634
3505a2c
80e52de
8abb1c1
746f353
cae826c
1de582b
b57cbf5
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
if we throw it's a breaking change, and we need to wait for Jest 28. Thoughts on making it a warning for now so we can land it, then throw later?
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.
Sounds reasonable.
Should I hide it behind a flag? Or just delete it and rewrite it for warnings?
This question is related to tests, the code itself is just one line, but tests are completely different.
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'd do
console.warn
instead of throwing, make the unit tests mock out the console, while the e2e test is essentially the same except for the exit codeThere 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.
resolveConfigPath
has to be a pure function (noconsole.warn
there), because it is run multiple times.There is
hasDeprecationWarnings: boolean
flag somewhere. I will look into it later.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.
hasDeprecationWarnings
has no text associated with it.The best solution I found was to propagate
shouldLogWarnings
. But I'm not sure it is good enough.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.
The "second" usage is here: https://github.com/facebook/jest/blob/a22ed6592f772df6b19314d9f06a7285c3b0968b/packages/jest-config/src/index.ts#L287
It's reading jest config to determine if there are any additional projects defined.
And then it is called again for each project.
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.
@SimenB ^ this one is still not solved.
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.
gave that a whack with
6a52634
(#11922)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.
Ok, so you went with
shouldLogWarnings
. 👍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.
yeah, it was either that or store some list of the exact warning we had logged (a
Set
to dedupe) then only log after all configs were resolved. This seemed cleaner