-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Group common jest config in to a preset #46953
Conversation
e00b567
to
8787b52
Compare
This PR does not affect the size of JS and CSS bundles shipped to the user's browser. Generated by performance advisor bot at iscalypsofastyet.com. |
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 don't fully understand the implications, but I don't have any objection WRT the composite-checkout package. The tests still pass, so I'm happy. Do you also need to convert the jest.config.js
in packages/shopping-cart
?
Yes, the |
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 new common config works for me very well! 👍
By default,
rootDir
is__dirname
, no need to specify it again
I tried to get rid of rootDir
some time ago, but failed. It was because I was trying to extend a config from another module:
const commonPreset = require( '../common/jest.config.js' );
module.exports = {
...commonPreset,
foo
}
But that didn't work and revealed a Jest bug. Extending the preset is much better!
Note: ICFY shows a reduction in bundle size but it is not real. It assumes this branch is targeting master, but it is not. |
That's true, ICFY always looks for a branching point from |
704d376
to
84088c3
Compare
Creates a
jest
preset to have the config for all packages in a common place.After reading Jest docs and source code, there are a few points that have influenced this decision:
Files referenced by
project
doesn't imply any type of extension. The config on the "parent"jest.config.js
is not propagated to files referenced byproject
. Reading config and projects is handled in https://github.com/facebook/jest/blob/master/packages/jest-config/src/index.ts#L273The only mechanism provided by
jest
to do extensions arepresets
. However it is only one-level: a preset can't have presets. Eg: https://github.com/facebook/jest/blob/master/packages/jest-config/src/normalize.ts#L519 is called when normalizing a config file, butsetupPreset
is not recursive.By default,
rootDir
is__dirname
, no need to specify it again (https://github.com/facebook/jest/blob/master/packages/jest-config/src/readConfigFileAndSetRootDir.ts#L96)Testing instructions