[CDX-404] Update packages and jsdom version#268
Conversation
There was a problem hiding this comment.
Pull request overview
Updates the test/runtime dependency set to address npm audit findings, including moving the Jest JSDOM environment forward (bringing in a newer jsdom) and adjusting tests/environment behavior to accommodate jsdom’s stricter window.location handling.
Changes:
- Upgrade
jest-environment-jsdomto^30.3.0(pulling injsdom@26.x) and update lockfile accordingly. - Add a custom Jest environment module that patches jsdom navigation behavior for
window.location.hrefassignments. - Refactor
shopifyDefaultstests to stop replacingwindow.locationand instead setwindow.location.hrefdirectly.
Reviewed changes
Copilot reviewed 3 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| spec/utils/shopifyDefaults.test.ts | Updates tests to avoid redefining window.location and use window.location.href instead. |
| spec/jest-env-jsdom.js | Adds a custom environment side-effect patch to make window.location.href = ... update the document URL rather than throw. |
| jest.config.js | Switches the client project to use the custom jsdom environment module. |
| package.json | Bumps jest-environment-jsdom version. |
| package-lock.json | Updates dependency graph for the new Jest/jsdom environment version. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1,12 @@ | |||
| const JSDOMEnvironment = require('jest-environment-jsdom').default; | |||
Code Review Results✅ StrengthsThe approach of replacing the unsafe 🚨 Critical Issues
|
TarekAlQaddy
left a comment
There was a problem hiding this comment.
I guess we have to keep in mind the jsdom internal patching just in case it ever changes in future versions
Ran
npm audit fixto get rid of security vulnerabilities. I had to upgrade jsdom to version 30 which is a breaking change with major differences.jest-environment-jsdomfrom^29.7.0to^30.3.0(pulls injsdom@26)Object.defineProperty(window, 'location', ...)which is no longer allowed in jsdom 26Why do we need a custom jsdom environment?
jsdom 26 does not support
window.location.href = '...'— it throws"Not implemented: navigation (except hash changes)". This is a long-standing jsdom limitation, but in previous versions our tests worked around it by replacingwindow.locationentirely with a plainURLobject (where.hrefis just a writable property, not a navigation trigger).With jsdom 26 locking down
window.location, we can no longer swap it out. Our source code (urlHelpers.ts,shopifyDefaults.ts) intentionally assigns towindow.location.hrefto navigate, and multiple tests exercise that code path. Without the patch, those tests would fail with the "Not implemented: navigation" error.Let me know if there is an easier approach to this 🤔