-
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
Fix/jest resolve module #47180
Fix/jest resolve module #47180
Conversation
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. |
d9574cd
to
032108f
Compare
Caution: This PR affects files in the Editing Toolkit Plugin on WordPress.com D52363-code has been created so you can easily test it on your sandbox. See this FieldGuide page about developing the Editing Toolkit Plugin for more info: PCYsg-ly5-p2 |
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.
This works great for me, even together with #46328. Thanks 👍
There are some suggestions to fix typos and add code comments, but other than that, the PR is ready.
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.
Looks good, thanks!
Background
#47002 introduced a change where we don't compile
packages/
as CJS by default. In those packages, in their package.json usuallymain
points to a CJS compilation andmodule
to a ESM.This is fine when those packages are used in webpack, as it will read the file from
module
. However Jest only looks formain
, so the tests will fail to resolve those modules.Change
This PR changes our custom Jest resolver to look for
module
and thenmain
if the package comes frompackages/
.Testing instructions
add/use-lp-in-calypso
, and verify all tests pass when you runyarn test-client
.