Node 24 Support by Shadowghost · Pull Request #1357 · git-for-windows/setup-git-for-windows-sdk

Shadowghost · 2026-03-16T07:52:13Z

Since GitHub will force Node 24 on June 2nd, 2026:

Update to Node 24 and npm 11
Update dependencies
Bump action version to 2

ESlint needs a bump once the GitHub plugin supports v10

dscho

@Shadowghost thank you for working on this! Quite a bit of work you had to do there before the PR build passed, thanks for seeing it through!

I am a bit concerned, though: While I agree that it would be good to move from Node.JS v20 to v24, I am not on board switching away from ncc and Jest without good reason. And I don't see any good reason.

dscho · 2026-03-16T13:06:19Z

    "format-check": "prettier --check **/*.ts",
    "lint": "eslint **/*.ts",
-    "package": "ncc build --source-map",
+    "package": "esbuild main.ts --bundle --platform=node --target=node24 --format=esm --sourcemap --outfile=dist/index.js",


Hmm. I have no experience whatsoever with esbuild, but tons with ncc. And I work in other projects that seem not to have any problems with ncc building ECMAScript Modules... (see e.g. here).

Changing the bundling system to something completely different was more than I bargained for when I started reviewing a PR to upgrade Node.JS v20 to v24...

dscho · 2026-03-16T13:09:44Z

@@ -1,4 +1,5 @@
 /* eslint no-console: "off" */
+import {test, expect} from 'vitest'


I have tons of experience with Jest, but none with vitest.

Changing the test framework as part of a PR to upgrade the minimum Node.JS version from v20 to v24 was not quite what I had in mind...

Shadowghost · 2026-03-16T14:25:07Z

Did a bit more testing:

Technically we don't need to forgo ncc, but esbuild output is 2.4MB vs 3.1MB on ncc - we can revert if you prefer that
Using jest did not work properly with the mockin/spying currently used in the tests - because it tries to operate on frozen esm namespaces

The main reason why this is an issue are the upgraded @action dependencies - they only export esm modules

dscho · 2026-03-16T16:15:05Z

    "format-check": "prettier --check **/*.ts",
    "lint": "eslint **/*.ts",
-    "package": "esbuild main.ts --bundle --platform=node --target=node24 --format=esm --sourcemap --outfile=dist/index.js",
+    "package": "esbuild main.ts --bundle --platform=node --target=node24 --format=esm --sourcemap --outfile=dist/index.js --banner:js=\"import { createRequire } from 'module'; const require = createRequire(import.meta.url);\"",


This is in line with what Claude Opus warns me about when switching from ncc to esbuild:

ncc is the conservative, purpose‑built choice for JS/TS GitHub Actions.

esbuild can work, but only if you are disciplined about CJS output, externals, and runtime testing of the bundled artifact.

The most common breakages when switching are dynamic require(), optional deps, and CJS/ESM interop — exactly the things Actions hit.

If the motivation is speed, esbuild is attractive. If the motivation is reliability, ncc still wins for Actions.

Why GitHub Actions are special

GitHub Actions (Node 20/22 runtime) have these properties:

Node-only execution (no browser assumptions)

Single entrypoint (dist/index.js)

Heavy use of:

require() (directly or indirectly via deps)

optional dependencies

conditional imports based on platform / Node version

You ship one JS file into the repo (no node_modules)

This is exactly the problem ncc was designed for:

“Compile a Node.js project into a single file. Supports TypeScript, binary addons, dynamic requires.” [github.com], [npmjs.com]

esbuild is more general‑purpose and faster, but it does not optimize for these Node edge cases.

The big gotchas when replacing ncc with esbuild (for Actions)

1. You must emit CommonJS

For GitHub Actions, CJS is the safe choice.

Real‑world guidance from Node+esbuild users:

When bundling for --platform=node and mixing CJS/ESM deps, --format=cjs works in most cases; --format=esm frequently triggers runtime failures. [dev.to]

If someone proposes:

esbuild --platform=node --format=esm

that’s a red flag for Actions unless they can prove runtime parity.

What to look for in the PR

--platform=node

--format=cjs

No "type": "module" surprises

2. Dynamic require() is the #1 Action killer

This is the classic failure mode:

“Dynamic require of '' is not supported”
occurs when a require() survives into the bundle and esbuild can’t statically resolve it. [github.com], [stackoverflow.com]

Why Actions hit this often:

@actions/* packages

optional deps (e.g. fsevents, platform‑specific stuff)

libraries that do require(someVariable)

ncc explicitly advertises handling this class of problem. esbuild does not guarantee it.

Required mitigation if using esbuild

Keep output CJS

Be very explicit about external vs bundled deps

Test the bundled file under node dist/index.js, not just unit tests

3. Native addons / non-JS assets

ncc explicitly claims support for binary addons.
esbuild does not magically solve this. [github.com]

If your Action (or its deps) touch:

.node binaries

wasm

JSON schemas loaded via fs

templates / certificates / text assets

then esbuild requires manual handling (copy steps, externals, loaders).

Many “it works locally” Action failures come from missing runtime files.

4. “Single file” still means runtime behavior must match

GitHub Actions don’t care how you bundle — only that runs.main works.

Common esbuild migration mistake:

✅ build succeeds

✅ tests pass against TS sources

❌ Action fails at runtime because:

optional dep path changed

__dirname assumptions broke

dynamic require wasn’t traced

ncc users hit this less often because the tool is Node‑deployment‑biased by design.

Is esbuild “safe” for Actions?

From a security / maintenance standpoint:

No direct vulnerabilities found in Snyk’s database for esbuild (latest versions). [security.snyk.io]

esbuild is very actively released (frequent version updates). [security.snyk.io]

It ships platform‑specific binaries (e.g. @esbuild/win32-x64), which is normal but sometimes triggers supply‑chain scrutiny in orgs. [socket.dev]

So “safe” in the OSS sense: yes.
“Safe” in the GitHub Action runtime‑parity sense: only if configured carefully.

Maintenance comparison (relevant to Actions)

esbuild: extremely active, fast releases, core infrastructure tool. [security.snyk.io]

ncc: still maintained, but slower cadence; narrower scope; very stable API. [github.com], [github.com]

For Actions, stability > features is usually the right trade.

My recommendation (Action‑specific)

Stick with ncc if:

The Action is customer‑facing or security‑sensitive

You rely on optional deps or Node ecosystem quirks

The current build is boring and works

Consider esbuild if:

Build time matters (monorepo, many Actions)

You are willing to:

lock output to CJS

add runtime smoke tests on the bundled file

own the occasional Node edge case

Absolute minimum bar for an esbuild PR

I’d insist on:

--platform=node --format=cjs

A CI step that runs the bundled output (node dist/index.js)

Explicit justification for any external deps

A rollback plan (ncc config kept around)

The way I read this is: Switching to esbuild would buy me a lot of trouble.

Shadowghost · 2026-03-26T08:51:27Z

@dscho reverted back to ncc

We are about to make a ton of changes to this file, so let's start from a current slate. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

The `::set-output` workflow command was deprecated in October 2022 in favor of writing to the `` environment file. See https://github.blog/changelog/2022-10-11-github-actions-deprecating-save-state-and-set-output-commands/ Assisted-by: Claude Opus 4.6 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

The eslint-plugin-github v6 upgrade (merged earlier via Dependabot in commit f513d0c) introduced a regression: it re-exports the `import` plugin internally, so the previous explicit `importPlugin.flatConfigs .typescript` entry now causes a "Cannot redefine plugin" error. This commit removes that redundant import-plugin plumbing and instead configures the import resolver through the `settings` block, adding `eslint-import-resolver-typescript` so that eslint can follow TypeScript path mappings. While here, the `@stylistic/func-call-spacing` rule name is updated to `@stylistic/function-call-spacing` to match the rename in @stylistic/eslint-plugin v5, and the `tseslint.config()` call is changed from array to variadic form as the array overload was deprecated. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

The upgraded @actions/core v3, @actions/cache v6, and @octokit/rest v22 packages ship ESM-only modules. This forces the project to switch to ESM (`"type": "module"` in package.json, `module: "nodenext"` in tsconfig.json), which in turn breaks Jest's mocking infrastructure. The fundamental problem is that ESM module namespaces are frozen (sealed) objects whose properties cannot be reassigned. Jest's `jest.mock()` and `jest.spyOn()` work by intercepting CommonJS `require()` calls and replacing properties on the returned module object, which is impossible with ESM namespaces. Jest 30 does offer `jest.unstable_mockModule()` as an ESM-aware alternative, but as the name indicates, it is explicitly unstable and requires passing the `--experimental-vm-modules` flag to Node.js at runtime. Node.js 24's built-in `node:test` runner was also considered. Its `mock.module()` API is similarly experimental (requiring `--experimental-test-module-mocks`) and does not support partial mocking of ESM modules at all: there is no way to replace a single named export (like `fs.rmSync`) while keeping the rest of the module's real implementations, which is a pattern this project's tests rely on heavily. Vitest is the only framework where ESM module mocking is a stable, first-class feature. Its `vi.mock()` intercepts at the module loader level rather than trying to modify frozen namespace objects. Vitest's test API (`describe`, `test`, `expect`, `beforeEach`) and mock API (`vi.mock`, `vi.spyOn`, `vi.fn`) deliberately mirror Jest's conventions, so the tests read almost identically to their Jest equivalents. Because this commit also excludes test files from the main `tsconfig.json` (to avoid emitting compiled test code into `lib/`), a separate `tsconfig.eslint.json` is added that extends the main config but re-includes test files and `.mjs` files. A `typecheck` npm script and corresponding CI wiring are not added yet at this stage because the test files use top-level `await` and vitest's own type declarations require `moduleResolution: "nodenext"`, neither of which works under the current `module: "commonjs"` setting. The typecheck script will be introduced in the subsequent commit that switches the project to ESM. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

The upgraded @actions/* dependencies (v3+) are ESM-only, so the project must use ESM imports. This switches the TypeScript compiler to `module: "nodenext"` and `moduleResolution: "nodenext"`, adds `"type": "module"` to package.json, and updates all local import specifiers to include the `.js` extension as required by the ESM resolution algorithm. The test file dynamic imports (`await import('../git')` etc.) gain the same `.js` suffix so that the type-checker's `nodenext` resolution accepts them. The compilation target is bumped from `es6` to `es2022` because the test files use top-level `await`, which requires `target` `es2017` or higher in the `tsconfig.eslint.json` type-check pass. Targeting `es2022` is the natural companion to `module: "nodenext"` and will also be needed by the @octokit/rest v22 type definitions in the following dependency-upgrade commit. The eslint `ignores` block is moved to a separate top-level config object so it takes effect globally rather than being scoped to a single override, which is needed because eslint-plugin-github sets defaults that would otherwise shadow the ignore list. Now that the module system supports it, this commit also adds the `typecheck` npm script (`tsc --noEmit -p tsconfig.eslint.json`) and wires it into both CI workflows and the `all` script, as announced in the preceding vitest commit. The tsconfig.eslint.json file gains `skipLibCheck: true` because vitest 4's own type declarations contain a type conflict (`Assertion<T>` simultaneously extends `JestAssertion<T>` and `ChaiMockAssertion` whose `lastReturnedWith` properties are not identical). Without `skipLibCheck`, this third-party declaration error would fail the type-check pass even though our own code is correct. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

A missing space ("Git for WindowsTypeScript") has been lurking in the package description since the project was first created from the TypeScript action template. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

The `node-fetch` package was introduced in commit a54551d ("Use node-fetch for fetchJSONFromURL") to download Azure Pipelines artifacts. Commit 18233c6 ("Do not use any Azure Pipelines artifact anymore", 2022-07-07) removed the last source-level import, switching all download paths to `curl.exe` piped to `tar.exe` (in `ci_artifacts.ts`) and to Git clones (in `git.ts`). The `@adobe/node-fetch-retry` wrapper was added in 6598596 and became equally dead at the same time. Neither package has been imported anywhere in the source since then. Removing them now avoids carrying dead install weight and, more importantly, side-steps the upcoming ESM migration headache: node-fetch v2 is CJS-only and node-fetch v3 is ESM-only, so either version would create friction in the ESM transition for zero benefit. Node 24 ships a native `fetch` global anyway. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

Bump all runtime and dev dependencies to their latest compatible versions. The three major runtime bumps (@actions/cache v5 to v6, @actions/core v2 to v3, and @octokit/rest v20 to v22) are the ones that require the ESM migration introduced in the preceding commits, as these new major versions ship ESM-only packages. The TypeScript compilation target was already bumped to `es2022` in the ESM commit because @octokit/rest v22's type definitions reference `ErrorOptions`, which is only available in the ES2022 standard library. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

GitHub will stop supporting the Node 20 runtime for GitHub Actions on June 2nd, 2026 (see https://github.blog/changelog/2025-12-05-github-actions-update-on-node20-deprecation/), so the `runs.using` field in action.yml is switched from `node20` to `node24`. Both CI workflows gain an explicit `actions/setup-node@v6` step pinned to `node-version: 24` so that `npm ci`, the build, and the test suite all run under the same Node version that the action will use at runtime. Without this step, CI would use whatever Node version the runner image happens to ship, which may not yet be v24. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

The switch to the Node 24 runtime is a breaking change for any consumer that pins to `@v1`, because GitHub will no longer execute that tag's `dist/index.js` once the Node 20 runtime is retired. A major version bump signals this cleanly: existing `@v1` users keep working until the runtime sunset, while new consumers and upgraders reference `@v2`. The README is updated accordingly: the usage example now references `@v2`, the recommended Node version for local development is changed from the long-outdated "Node 12" to "Node 24", and the example test output is refreshed to reflect the vitest output format. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

Regenerate the ncc-bundled `dist/index.js` and its source map to reflect all the preceding source changes. The bundle is now an ESM entry point (importing `./sourcemap-register.cjs` and using `createRequire` for any remaining CJS interop) rather than the previous CommonJS bundle, matching the `"type": "module"` declared in `dist/package.json`. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

dscho · 2026-04-24T13:55:02Z

I spent a substantial time with Claude Opus to rewrite this into a commit history that even somebody like me could review confidently. Before we merge this, the CI has to pass, of course, but I also want to merge #1359 first, so that the last v1 will still be secure by default.

Shadowghost requested a review from dscho as a code owner March 16, 2026 07:52

Shadowghost mentioned this pull request Mar 16, 2026

Node 24 Support #1356

Open

Shadowghost force-pushed the node-24 branch 3 times, most recently from b5324b8 to 9bfb088 Compare March 16, 2026 08:34

dscho reviewed Mar 16, 2026

View reviewed changes

Shadowghost requested a review from dscho March 26, 2026 08:51

dscho and others added 11 commits April 24, 2026 15:41

Regenerate the package-lock.json file

484f27d

We are about to make a ton of changes to this file, so let's start from a current slate. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

Fix typo in package.json description

14806b9

A missing space ("Git for WindowsTypeScript") has been lurking in the package description since the project was first created from the TypeScript action template. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

dscho force-pushed the node-24 branch from 4f51569 to 538dfa8 Compare April 24, 2026 13:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Node 24 Support#1357

Node 24 Support#1357
Shadowghost wants to merge 11 commits intogit-for-windows:mainfrom
Shadowghost:node-24

Shadowghost commented Mar 16, 2026 •

edited

Loading

Uh oh!

dscho left a comment

Uh oh!

dscho Mar 16, 2026

Uh oh!

dscho Mar 16, 2026

Uh oh!

Shadowghost commented Mar 16, 2026

Uh oh!

dscho Mar 16, 2026

Uh oh!

dscho Mar 16, 2026

Uh oh!

Shadowghost commented Mar 26, 2026

Uh oh!

dscho commented Apr 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		@@ -1,4 +1,5 @@
		/* eslint no-console: "off" */
		import {test, expect} from 'vitest'

Conversation

Shadowghost commented Mar 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dscho left a comment

Choose a reason for hiding this comment

Uh oh!

dscho Mar 16, 2026

Choose a reason for hiding this comment

Uh oh!

dscho Mar 16, 2026

Choose a reason for hiding this comment

Uh oh!

Shadowghost commented Mar 16, 2026

Uh oh!

dscho Mar 16, 2026

Choose a reason for hiding this comment

Why GitHub Actions are special

The big gotchas when replacing ncc with esbuild (for Actions)

1. You must emit CommonJS

2. Dynamic require() is the #1 Action killer

3. Native addons / non-JS assets

4. “Single file” still means runtime behavior must match

Is esbuild “safe” for Actions?

Maintenance comparison (relevant to Actions)

My recommendation (Action‑specific)

Stick with ncc if:

Consider esbuild if:

Absolute minimum bar for an esbuild PR

Uh oh!

dscho Mar 16, 2026

Choose a reason for hiding this comment

Uh oh!

Shadowghost commented Mar 26, 2026

Uh oh!

dscho commented Apr 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Shadowghost commented Mar 16, 2026 •

edited

Loading

2. Dynamic `require()` is the #1 Action killer