-
Notifications
You must be signed in to change notification settings - Fork 3.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: add yarn v2 pnp support to default webpack processor #17335
fix: add yarn v2 pnp support to default webpack processor #17335
Conversation
Thanks for taking the time to open a PR!
|
hmm, looks like the failing CI is caused by network issue?
Can someone help to retry the job? |
@jennifer-shehane @flotwig since you're marked as reviewers.... anything else i can do to get this merge? |
Nothing right now. We are working on getting 8.0 out the door right now, but this is a priority for me to get reviewed after 8.0 is out. |
Is there anything I can do locally to make the pnp API work until this gets released? Id like to try 8.0, but this blocks me |
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.
There're 2 ways to make Cypress support yarn v2 natively (and close #8008)
1. Switch to use webpack5 from default webpack preprocessor which has built-in pnp support. 2. Add `pnp-webpack-plugin` to our `@cypress/webpack-batteries-included-preprocessor` once [Fix #32 try to use findPnpApi to locate pnpapi arcanis/pnp-webpack-plugin#37](https://github.com/arcanis/pnp-webpack-plugin/pull/37)
I can follow up on option 2
Upgrading the preprocessor to Webpack 5 would be a better solution long-term, but it would have to wait until Cypress 9 since it would be a breaking change. If you want to proceed with #2, that would be a good change to pull in for now that would be non-breaking.
Other than that, tested manually with a local yarn v2 project and yarn node
now seems to properly set up NODE_OPTIONS
for the plugins process again.
Thanks. ping @arcanis any chance we can get arcanis/pnp-webpack-plugin#37 merged? |
Thanks @arcanis, arcanis/pnp-webpack-plugin#37 is merged and i am going to add it to our default webpack profile so we can support pnp natively. |
@flotwig @jennifer-shehane I've added pnp-webpack-plugin to our Let me know if there's anything i should do. |
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 to me, @chrisbreiding can you take a quick look now that this is going to modify the behavior of the webpack preprocessor?
@missing1984 Thank you for the contribution! |
User facing changelog
Add Yarn v2 pnp support to our default webpack processor.
Additional details
This MR addressed couple issues around yarn v2 pnp support.
@cypress/webpack-batteries-included-preprocessor
so we can support pnp out of the box. Cypress and Yarn 2 PNP support #8008How has the user experience changed?
Once this merged, Yarn v2 pnp should work with Cypress out of the box
PR Tasks
cypress-documentation
?type definitions
?cypress.schema.json
?