NodeJS 24 Support #59
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Chapman Pendery <[email protected]>
* Port to NAPI (microsoft#644) * Port to NAPI The "5th pty bug" in microsoft#432 fixed also. * Fix help message in pty.cc * Move NAPI deps to devDependencies in package.json * Apply most of deepak1556's suggestions * Fix winpty * Fix conpty missing CloseHandle * Use unique_ptr to avoid `goto`s * Why macos failed? * fix: ci and minor cleanups * fix build failed on windows --------- Co-authored-by: deepak1556 <[email protected]> * build(deps): bump ip from 2.0.0 to 2.0.1 Bumps [ip](https://github.com/indutny/node-ip) from 2.0.0 to 2.0.1. - [Commits](indutny/node-ip@v2.0.0...v2.0.1) --- updated-dependencies: - dependency-name: ip dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]> * api scan * fix job name * chore: fix APIScan software name (microsoft#667) * fix: comment out invalid API call (microsoft#669) * fix: assertion on node environment shutdown (microsoft#672) * Upgrade node-gyp to fix microsoft#643 and microsoft#646 (microsoft#673) * chore: remove deprecated api `process.binding` (microsoft#653) * Remove deprecated API `process.binding` Originally designed to work with ancient node.js 0.12 and io.js * Fix node `net.Socket` limitations nodejs/node#37780 * chore: migrate pipeline to use 1ES template (microsoft#676) * build(deps): bump tar from 6.2.0 to 6.2.1 Bumps [tar](https://github.com/isaacs/node-tar) from 6.2.0 to 6.2.1. - [Release notes](https://github.com/isaacs/node-tar/releases) - [Changelog](https://github.com/isaacs/node-tar/blob/main/CHANGELOG.md) - [Commits](isaacs/node-tar@v6.2.0...v6.2.1) --- updated-dependencies: - dependency-name: tar dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]> * docs: add a note on Spectre-mitigated libraries (microsoft#679) * build(deps): bump braces from 3.0.2 to 3.0.3 Bumps [braces](https://github.com/micromatch/braces) from 3.0.2 to 3.0.3. - [Changelog](https://github.com/micromatch/braces/blob/master/CHANGELOG.md) - [Commits](micromatch/braces@3.0.2...3.0.3) --- updated-dependencies: - dependency-name: braces dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]> * Include termios.h on freebsd * Get conpty.dll conditionally loading * Add conpty dlls and node headers * Add useConptyDll option * Copy conpty.dll and openconsole.exe depending on arch in postinstall * Ship third_party folder with module This needs to be there to copy over the right binary on postinstall Part of microsoft/vscode#224488 * Don't run CI for node 16 * Disable APIScan task * Fix winpty resize and reduce test flakiness * Publish third_party folder * Improve can't find conpty.dll exception message Part of microsoft/vscode#224488 * Get path of conpty.node for conpty.dll Part of microsoft/vscode#224488 * Update to newer version of conpty * Fix correct method being loaded when not using conptydll Part of microsoft/vscode#224488 * Copy right dll/exe based on npm_config_arch Part of microsoft/vscode#224488 Part of microsoft/vscode#225355 * Bring back clear impl Fixes microsoft#711 * Close piClient.hThread handle Fixes microsoft#717 * Move buffer_ to heap Resolves warning: src\win\path_util.cc(54): warning C6262: Function uses '131804' bytes of stack: exceeds /analyze:stacksize '16384'. Consider moving some data to heap. * Catch possible exceptions in ~Agent Fixes warning: deps\winpty\src\agent\Agent.cc(231): warning C4722: 'Agent::~Agent': destructor never returns, potential memory leak * yarn.lock -> package-lock.json Fixes microsoft#712 * Regenerate dependencies * Fix typo in trace log Follow up microsoft#720 * Move to compile commands on postinstall for fixing intellisense Fixes microsoft#707 * chore: use node 20 and fix build (microsoft#730) * chore: use node 20 and fix build * chore: bump macOS deployment target * spec: reduce flakyness * Revert "chore: bump macOS deployment target" This reverts commit d9d18b3. --------- Co-authored-by: deepak1556 <[email protected]> * chore: add new publish pipeline (microsoft#727) Also allows the CI pipeline to use the Unofficial template * fix: ensure proper cleanup of console process on shutdown * fix: publish pipeline broken APIScan task (microsoft#756) * chore: improve cleanup with useConptyDll mode * chore: update tests * chore: auto-publish beta versions (microsoft#757) * chore: work around blocked release integration (microsoft#758) * Update to conpty v1.22 (microsoft#759) * Update to conpty v1.22 Fixes microsoft#490 * Add logs to show the problem happening * spec: increase exit delay * refactor: input and output handling with conpty * Close the input read and output write handles after creating the client process * Call ReleasePseudoConsole after creating the client process which will cause the output read handle to close when there is no more data from the session * For manual termination via Kill, we close the input write handle and call into ClosePseudoConsole, we should then drain the output handle NB: ideally draining the output handle should have been enough to cause the client process to close but it doesn't work, we call TerminateProcess to fix this case. * chore: restore legacy conpty path --------- Co-authored-by: deepak1556 <[email protected]> * chore: remove old publishing stage (microsoft#761) * fix: restore conpty non-dll path (microsoft#766) * chore: add BinSkim flags to winpty (microsoft#767) * chore: add BinSkim flags to winpty * Apply PR feedback * [email protected] * Change buffer size to 128KiB Fixes microsoft#765 * chore: match trigger with perf-bot's (microsoft#773) * chore: match exclude with latest conpty (microsoft#774) * chore: use folder wildcard (microsoft#775) * Revert "[email protected]" This reverts commit 247ae7d. * fix: gate conpty-exclusive call behind conpty check (microsoft#778) * NodeJS-24 * Node 20 --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: kkocdko <[email protected]> Co-authored-by: deepak1556 <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Daniel Imms <[email protected]> Co-authored-by: SteVen Batten <[email protected]> Co-authored-by: Raymond Zhao <[email protected]> Co-authored-by: jpcastberg <[email protected]>
bwp91
previously approved these changes
Jun 11, 2025
There was a problem hiding this comment.
Pull Request Overview
Adds support for Node.js 24 by updating ABI data, build targets, and pipelines; introduces a useConptyDll flag to Windows PTY logic and adjusts related tests.
- Add Node.js 24 entries to
abi_registry.jsonand override bad data with an update script - Extend Windows PTY agent and connection classes with a
useConptyDllflag - Update tests to cover new
useConptyDllscenarios and adjust CI workflows to include Node 24
Reviewed Changes
Copilot reviewed 22 out of 33 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| .prebuild/updateABIRegistry.js | Copies corrected ABI registry for Node 24 before builds |
| build.sh | Updates Node build targets (adds -t 24.0.0) but has an unconditional exit |
| src/windowsConoutConnection.ts | Adds _useConptyDll flag and changes dispose logic |
| src/windowsPtyAgent.ts | Passes useConptyDll into native connect/kill and adjusts kill cleanup |
| src/windowsTerminal.test.ts | Expands test matrix to include useConptyDll, adds debug console.logs |
| azure-pipelines.yml | Locks pipeline to Node 20.x for build but later validation covers Node 24 |
Comments suppressed due to low confidence (4)
src/windowsPtyAgent.ts:201
- Disposal of the
ConoutConnectionwas removed from the non-conpty branch, which may leak the worker. Ensuredispose()is called for bothuseConptyfalse and true.
}
build.sh:51
- The unconditional
exit 1at the end will always signal failure. Remove it or make it conditional so that the script only exits with an error on real failures.
exit 1
src/windowsTerminal.test.ts:29
- [nitpick] These
console.logstatements clutter test output. Consider removing them or using a configurable debug logger for cleaner test results.
console.log('expected pids', JSON.stringify(pids));
src/windowsTerminal.test.ts:77
- [nitpick] This debug log is useful during development but may be too verbose for CI logs. Remove or guard behind a debug flag.
console.log('list', JSON.stringify(list));
bwp91
approved these changes
Jul 2, 2025
5 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Update approach was to update from node-pty, then fix forward
Also, needed to work around this issue - https://github.com/electron/node-abi/pulls