fix: patch-package is not applied in dist'ed build #19239
Conversation
|
Thanks for taking the time to open a PR!
|
|
Test summary
Run details
View run in Cypress Dashboard ➡️ Flakiness
This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
||||||||||||||||||||||||||
tgriesser
requested review from
jennifer-shehane,
brian-mann and
chrisbreiding
and removed request for
a team
| "engine.io": "5.0.0", | ||
| "engine.io-parser": "4.0.2", |
There was a problem hiding this comment.
Needed to add this, as it was being hoisted when devDeps are stripped & patch wasn't being applied.
| @@ -113,6 +113,7 @@ | |||
| "trash": "5.2.0", | |||
| "tree-kill": "1.2.2", | |||
| "ts-node": "8.5.4", | |||
| "tsconfig-paths": "3.10.1", | |||
There was a problem hiding this comment.
why did this need move from a dev-dep?
There was a problem hiding this comment.
Trying to find where it failed - might have only been locally when I was writing the test case for the patch being applied, and it wasn't... and then I noticed that we do indeed use the package in a prod situation:
cypress/packages/server/lib/util/ts_node.js
Lines 30 to 37 in 11e99fc
| @@ -68,15 +68,23 @@ export async function buildCypressApp (options: BuildCypressAppOpts) { | |||
| // Copy Packages: We want to copy the package.json, files, and output | |||
There was a problem hiding this comment.
why re-name from .patch to .dev.patch?
There was a problem hiding this comment.
Do the patches in packages/socket/patches & packages/server/patches also need to be renamed to .dev.patch?
There was a problem hiding this comment.
It's the way you're supposed to name them for devDependencies
There was a problem hiding this comment.
Oops, had the wrong link, updated the above with the failing circle job. But here's the docs: https://github.com/ds300/patch-package#dev-only-patches
And no, the socket & server ones are prod dependencies, so they remain .patch
tgriesser
dismissed stale reviews from emilyrohrbough and chrisbreiding
via
5bfb385
…text * 10.0-release: (45 commits) fix: various Nav Bar fixes (#19283) build: add patch package as a dev dependency for fe-shared chore: hoist is - fun with cached dependencies build: hoist is hard build: better hoisting strategy fix: remove windows and mac workflow from branch revert: remove change about node version 17 build: remove testing of desktop-gui assets build: run window & mac CI in this branch build: more fixes build: remove toycode mdi from launchpad rename patch because of dev dep build: fix merge issue in packages generation chore: update sass for windows compatibility fix: Do not crash when a ill formed URL request is proxied (#19274) fix: remove desktop-gui from circle.yml change whitepace in patch fix: adding timeout option to writeFile command (#19015) release 9.1.1 fix: patch-package is not applied in dist'ed build (#19239) ...
TypeError: data.hasOwnProperty is not a function#19091cy.request#19115~is converted to\x7e#19096User facing changelog
Due to changes to build process changes in #17285, the
patch-packagescripts were not being properly applied as they were previously. Fixes the build process to include thescriptsentries in both the root & packages for the yarn install, so the postinstall entries are applied properly.Added tests in
testStaticAssets.tsto ensure the correct patches are being applied.