-
Notifications
You must be signed in to change notification settings - Fork 200
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
Use newer version of JSDOM #3979
Conversation
9914e32
to
f0329a0
Compare
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.
Pull Request Overview
This PR updates the JSDOM version to support web components by switching to a custom Jest environment.
- Updates jest.config.ts to use the custom environment
- Introduces a new jest-environment-jsdom.ts file
- Updates the dead code detection script to ignore the new environment file
Reviewed Changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated no comments.
File | Description |
---|---|
extensions/ql-vscode/src/view/jest.config.ts | Updates testEnvironment to point to the custom Jest environment file |
extensions/ql-vscode/src/view/jest-environment-jsdom.ts | Adds a custom Jest environment implementation using a newer JSDOM |
extensions/ql-vscode/scripts/find-deadcode.ts | Excludes the newly added Jest environment file from dead code detection |
Files not reviewed (1)
- extensions/ql-vscode/package.json: Language not supported
Tip: If you use Visual Studio Code, you can request a review from Copilot before you push from the "Source Control" tab. Learn more
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.
LGTM if this helps us move forwards with the vscode/elements migration. Maybe it's not ideal being on an alpha version but given we only use it for testing it can hopefully only cause stability issues for us and not for users 🤷🏼
"jest-runner-vscode": "^3.0.1", | ||
"jsdom": "^26.0.0", |
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.
Looking at jest-environment-jsdom
it seems we're going jsdom
version 20 to 26.
Looking in the changelog I can't immediately see any references to the fields that weren't defined and were causing errors, but it could be I missed it or it's from another updated dependency 🤷🏼
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.
I believe what we're looking for is in version 23.1.0: "Added an initial implementation of ElementInternals
, including the shadowRoot
getter and the string-valued ARIA properties." This is what was giving errors when using some @vscode-elements/elements
components (after fixing a missing CSSProperties.replaceSync
definition).
To prepare for upgrading to
@vscode-elements/elements
, we need a newer version of JSDOM that better supports web components. This upgrades to a newer version of JSDOM by using a custom Jest environment that is available in Jest 30.