Skip to content
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

patch-package not correctly applied since Yarn upgrade #6582

Closed
flotwig opened this issue Feb 27, 2020 · 5 comments · Fixed by #6583 · May be fixed by rangle/test-automation#29
Closed

patch-package not correctly applied since Yarn upgrade #6582

flotwig opened this issue Feb 27, 2020 · 5 comments · Fixed by #6583 · May be fixed by rangle/test-automation#29
Assignees
Labels
type: regression A bug that didn't appear until a specific Cy version release v4.0.2 🐛 Issue present since 4.0.2

Comments

@flotwig
Copy link
Contributor

flotwig commented Feb 27, 2020

patch-package has not been applied to built binaries ever since #5555 was merged, leading to regressions coming from an unpatched has-binary2 when users send certain circular data over WebSockets:

RangeError: Maximum call stack size exceeded
at get (internal/bootstrap/pre_execution.js:286:8)
at hasBinary (/builds/platform/ui/client/cache/Cypress/4.0.2/Cypress/resources/app/packages/socket/node_modules/has-binary2/index.js:44:3)
at hasBinary (/builds/platform/ui/client/cache/Cypress/4.0.2/Cypress/resources/app/packages/socket/node_modules/has-binary2/index.js:58:59)
at hasBinary (/builds/platform/ui/client/cache/Cypress/4.0.2/Cypress/resources/app/packages/socket/node_modules/has-binary2/index.js:58:59)
at hasBinary (/builds/platform/ui/client/cache/Cypress/4.0.2/Cypress/resources/app/packages/socket/node_modules/has-binary2/index.js:58:59)

The patch is correctly applied during development, but during production builds it is not. This is probably because has-binary2 is in node_modules in dev, but in packages/socket/node_modules in the built binary.

@flotwig flotwig self-assigned this Feb 27, 2020
@flotwig flotwig added the v4.0.2 🐛 Issue present since 4.0.2 label Feb 27, 2020
@jennifer-shehane jennifer-shehane added the type: regression A bug that didn't appear until a specific Cy version release label Feb 27, 2020
@flotwig
Copy link
Contributor Author

flotwig commented Feb 27, 2020

This patch in driver does seem to still be applied correctly in both production and development:

https://github.com/cypress-io/cypress/blob/cb0f32b0b4913cbb403f2e7c51b23ecad50ece9f/packages/driver/patches/sinon+8.1.1.patch#L1-L20

I believe this is a coincidence though, there is an earlier version of sinon installed in the root node_modules which causes this sinon to be installed in the driver folder in both development and production.

node_modules in development and production should probably be aligned and then we can have confidence in where the patches will be applied every time.

@bahmutov
Copy link
Contributor

bahmutov commented Feb 27, 2020 via email

@flotwig
Copy link
Contributor Author

flotwig commented Feb 27, 2020

I think we just need to add the has-binary2 dep to nohoist, then it will work (turns out @packages/driver/sinon is also in nohoist, which is why the patch works)

@flotwig flotwig mentioned this issue Feb 27, 2020
2 tasks
@cypress-bot cypress-bot bot added stage: needs review The PR code is done & tested, needs review and removed stage: work in progress labels Feb 27, 2020
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Feb 28, 2020

The code for this is done in cypress-io/cypress#6583, but has yet to be released.
We'll update this issue and reference the changelog when it's released.

@cypress-bot cypress-bot bot removed the stage: needs review The PR code is done & tested, needs review label Feb 28, 2020
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Feb 28, 2020

Released in 4.1.0.

This comment thread has been locked. If you are still experiencing this issue after upgrading to
Cypress v4.1.0, please open a new issue.

@cypress-bot cypress-bot bot locked as resolved and limited conversation to collaborators Feb 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: regression A bug that didn't appear until a specific Cy version release v4.0.2 🐛 Issue present since 4.0.2
Projects
None yet
3 participants