Add shorthand for watch plugins and runners #7213
Conversation
packages/jest-config/src/utils.js
Outdated
| }); | ||
|
|
||
| if (!module) { | ||
| throw createValidationError( |
There was a problem hiding this comment.
seems like you could throw this in resolveWithPrefix? They're all identical except for, in this case, Test environment and 'testEnvironment'.
So you could end up with e.g.
export const getTestEnvironment = ({
rootDir,
testEnvironment: filePath,
}: Object) => {
return resolveWithPrefix(null, {
filePath,
prefix: 'jest-environment-',
rootDir,
optionName: 'testEnvironment',
humanOptionName: 'Test environment',
});
};or something like that
There was a problem hiding this comment.
I originally left it out because the watch plugin validation happens as an element in an array. But I guess it is fine to show the same message.
packages/jest-config/src/utils.js
Outdated
| export const getTestEnvironment = ({ | ||
| rootDir, | ||
| testEnvironment: filePath, | ||
| }: Object) => { |
There was a problem hiding this comment.
can we use a better type than Object here? I know you didn't introduce it
There was a problem hiding this comment.
Should be mentioned in the docs.
https://jestjs.io/docs/en/configuration#runner-string
Watch plugins are actually missing from https://jestjs.io/docs/en/configuration...
|
Flow errors :( |
packages/jest-runner/src/run_test.js
Outdated
| @@ -55,10 +55,10 @@ async function runTestInternal( | |||
|
|
|||
| let testEnvironment = config.testEnvironment; | |||
|
|
|||
| if (customEnvironment) { | |||
| if (customEnvironment && customEnvironment[0]) { | |||
There was a problem hiding this comment.
This was failing Flow because it was passing an array instead of a string(and before the flow type was more relaxed)
From what I understand this only workder because at some point in getTestEnviromnent we are doing jest-environment-${env} which if env was an array(jest-envrionment-${['my-env']}), it would just get stringified as jest-environment-my-env. Same story when it does require.resolve(${prefix}${fileName}).
Does this change make sense? From what I can think you can only pass one env per docblock
There was a problem hiding this comment.
Does this change make sense? From what I can think you can only pass one env per docblock
Yeah, it does! We might want to allow multiple envs in the future (to test in both node and the browser), but for now this is correct 🙂
There was a problem hiding this comment.
I might just add a @FlowFixMe because it seems that even though the type says Array<string> | string it seems to be a string when passing the docblock.
I'll investigate more later today because I have to head out.
|
Mind rebasing? :) |
|
Is this ready to land @rogeliog? I was holding off on merging due to #7213 (comment) |
|
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
This PR fixes #7156
Test plan
Tests were added.