-
Notifications
You must be signed in to change notification settings - Fork 15
feat!: upgrade support to node 22 and 24 #63
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
Merged
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
We are also upgrading the minimum engine to 22, and the dependencies we use for CodeQL to 24, as it's going to be our baseline from now on.
According to a v8 commit, v8 removed the ability to build without shared r/o heaps. More info: https://chromium-review.googlesource.com/c/v8/v8/+/6038531
We only use one EmbedderSnapshotData and one Isolate, so we are not hitting the corner case of having isolates with different snapshots.
addaleax
reviewed
Nov 1, 2025
In theory vcbuild.bat should detect where the MSVC DevTools are located (using vswhere.exe) and setup them correctly. We are going to verify this behaviour.
Compiling with a snapshot requires compiling twice: a base image and the image with the snapshot embedded. In that situation, clang-cl complains that there are precompiled headers that changed between compilations and does not refresh them, but kills the compilation process with an error. Due to this, before attempting the second compilation, we will delete all pch files.
addaleax
reviewed
Nov 12, 2025
This test was written so we could speed up the feedback loop.
Because we can keep adding more tests, maintaining the GHA workflow can become a bit cumbersome. This script generates the GHA file from a template, and adds all the test cases in a matrix. We do this because each test is expensive in time and disk space, so we do sometimes hit the GHA limit of 6h or the we fill the disk.
It seems that -g expects a regular expression, and if the regular expression is invalid, mocha just fails with Error: null. Using -f, uses a substring comparison, which is what we need.
nodeSourcePath in Windows refers to a user directory that contains an alias. For example, something like C:\Users\RUNNER~1 (in GHA) while MSVC uses the full name like C:\Users\runneradmin. This can make rimraf not delete the pch files properly. By resolving first, we make sure that both paths are absolute and unambiguous.
a92c9b1 to
9a03778
Compare
clang-cl does not properly handle pch refresh when a source header changed after the pch file was generated
70c2a90 to
5b9d8c2
Compare
This will force Node.js to compile from scratch. It's slower, but it will only happens on Windows and it will be safer and more reliable there.
Right now it's failing on the first compilation of the second test, as we are not doing any clean up. To ensure that we are in a consistent state in Windows, before attempting compiling, we will make sure we don't have any old compilation artifact. This might slow down Windows compilations as they can't reuse cached files.
addaleax
approved these changes
Nov 18, 2025
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.
We are also upgrading the minimum engine to 22, and the dependencies we use for CodeQL to 24, as it's going to be our baseline from now on.