-
Notifications
You must be signed in to change notification settings - Fork 1
Add support for testing Node 24 interoperability #76
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
Conversation
|
Beta Published - Install Command: |
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 adds interoperability tests for Node.js v24 and updates various test files, configuration, and version requirements to align with the new environment. It also applies consistent formatting (via Prettier) across code and updates workflows and dependencies.
- Add
node-24andes2026tests gated onprocess.version < 'v24' - Bump package and peer dependency versions; update CI/workflow node versions to 24.x
- Reformat many files to multiline calls for consistency
Reviewed Changes
Copilot reviewed 26 out of 27 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/typescript/typescript-5.*.spec.ts | Updated header comments & multiline formatting |
| src/test/node/node-22.spec.ts | Renamed file comment, expanded multiline calls |
| src/test/node/node-24.spec.ts | New tests for Node 24 (URLPattern, import.meta.main) |
| src/test/lib/index.ts | Import new es2026.spec.ts |
| src/test/lib/es2026.spec.ts | New ES2026 feature tests (Error.isError) |
| src/index.test.ts | Renamed comment to reference test entry |
| src/compile.ts | Multiline formatting for calls & error collection |
| src/compile.spec.ts | Multiline formatting for helper functions and calls |
| src/builder.ts | Multiline formatting around parseArgs destructure & writes |
| src/analyze.ts | Multiline formatting in reduction logic |
| src/analyze.spec.ts | Multiline formatting in helper functions |
| package.json | Version bump to 9.2.0; updated peer deps engines |
| eslint.config.mjs | Multiline formatting of .gitignore entries |
| README.md | Update minimum Node requirement to 22.15 |
| .github/workflows/{publish,publish-beta,ci}.yml | Updated node-version matrices to include 24.x |
Comments suppressed due to low confidence (2)
src/test/node/node-22.spec.ts:4
- The test uses
fileURLToPathbut it isn't imported. Addimport { fileURLToPath } from 'node:url';at the top.
import module from 'node:module';
src/test/typescript/typescript-5.6.spec.ts:28
- The logical OR is misplaced, causing
'loose'to always be truthy. It should be two separate calls:isValid(1, {}, 'loose') || isValid(2, {}, 'loose').
isValid(1, {}, 'loose' || isValid(2, {}, 'loose'))
ramaghanta
left a comment
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.
lftm
le-cong
left a comment
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.
only a minor one
| }); | ||
|
|
||
| // 24.2+ | ||
| it('import.meta.main is available', async () => { |
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.
it fails on node v24.0.0, should we make this test conditional?
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.
Since it's only ever being tested on the latest versions of 22.x/24.x (due to the way GitHub actions work) I haven't bothered with that in the past. I think it's fair to assume anyone running tests on this should be up-to-date, and if this reminds them to update their local node version, probably for the best. :)
|
❌ PR review status - has 1 reviewer outstanding |
le-cong
left a comment
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.
lftm
Fixes #75.
Some changes/additions to test, but no prod code changes other than prettier. This is in preparation for TS 5.9, which should be releasing soon.