feat(runner): support Node 20 via oxc-node TypeScript loader#1388
Open
Simon Ingeson (smonn) wants to merge 1 commit into
Open
feat(runner): support Node 20 via oxc-node TypeScript loader#1388Simon Ingeson (smonn) wants to merge 1 commit into
Simon Ingeson (smonn) wants to merge 1 commit into
Conversation
WalkthroughThis PR lowers the supported Node floor to 20.19.0, adds runtime capability detection and a loader strategy selector, and introduces an async Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant loadFlowDefault
participant registerFlowLoader
participant selectFlowLoaderStrategy
participant OxcNodeCore
loadFlowDefault->>registerFlowLoader: await registerFlowLoader()
registerFlowLoader->>selectFlowLoaderStrategy: selectFlowLoaderStrategy(caps)
selectFlowLoaderStrategy-->>registerFlowLoader: strategy
alt strategy is oxc-transpile
registerFlowLoader->>OxcNodeCore: import `@oxc-node/core/register`
else strategy is sync-alias
registerFlowLoader->>registerFlowLoader: register sync alias hook
else strategy is unsupported
registerFlowLoader->>loadFlowDefault: throw Error (Node 20.6+ required)
end
Related PRs: None identified from the provided summary. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Simon Ingeson (smonn)
force-pushed
the
wiz-10992-support-node-20
branch
from
3cfc3f0 to
31cc7f3
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/ci.yml (1)
46-99: 📐 Maintainability & Code Quality | 🔵 TrivialConsider dropping the Node 20 smoke lane
.github/workflows/ci.yml:46-99still exercisesnode-20, but Node 20 is EOL and no longer receives security fixes. If the Docker target can move, remove this matrix entry; otherwise document why an EOL runtime must stay in CI.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yml around lines 46 - 99, The runtime-smoke workflow still includes the node-20 matrix entry even though Node 20 is EOL. Update the matrix in the runtime-smoke job to remove the node-20 lane if the Docker target no longer depends on it, and keep the remaining node-22, node-24, and bun lanes intact; if you must retain Node 20, add an explicit workflow comment near the matrix in runtime-smoke explaining why the EOL runtime is still required.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 58-66: The CI matrix for the Node 20 job is using a floating major
version instead of the declared minimum supported floor. Update the node-20
entry in the workflow matrix to pin the exact minimum Node 20.x version used in
engines.node, and keep the other matrix entries unchanged so the smoke test
exercises the actual supported floor via the setup-node configuration.
In `@README.md`:
- Line 26: The Node support note in README should be updated because the
statement that Node 20 is in maintenance LTS is outdated. Rewrite the sentence
near the supported Node versions text to reflect that Node 20 is EOL or to
direct readers to the current supported LTS line, keeping the wording aligned
with the project’s actual runtime support.
---
Outside diff comments:
In @.github/workflows/ci.yml:
- Around line 46-99: The runtime-smoke workflow still includes the node-20
matrix entry even though Node 20 is EOL. Update the matrix in the runtime-smoke
job to remove the node-20 lane if the Docker target no longer depends on it, and
keep the remaining node-22, node-24, and bun lanes intact; if you must retain
Node 20, add an explicit workflow comment near the matrix in runtime-smoke
explaining why the EOL runtime is still required.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 35452280-245b-44d2-b1c5-32fd067f1e60
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (22)
.changeset/support-node-20.md.github/workflows/ci.yml.gitignore.oxlintrc.jsonCONTRIBUTING.mdREADME.mdpackage.jsonscripts/build.tsscripts/buildBinary.tssrc/domains/doctor/checks/nodeVersion.test.tssrc/domains/doctor/checks/nodeVersion.tssrc/domains/runner/loadFlowDefault.tssrc/oxcNode.d.tssrc/shell/resolver/detectRuntimeCapabilities.test.tssrc/shell/resolver/detectRuntimeCapabilities.tssrc/shell/resolver/registerFlowLoader.test.tssrc/shell/resolver/registerFlowLoader.tssrc/shell/resolver/selectFlowLoaderStrategy.test.tssrc/shell/resolver/selectFlowLoaderStrategy.tstest/node20/greeting.tstest/node20/sample.flow.tstest/node20/smoke.mjs
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/shell/resolver/selectFlowLoaderStrategy.ts`:
- Around line 37-41: The native TypeScript path in selectFlowLoaderStrategy
currently falls back to "none" when hasNativeTypeScript is true but hasSyncHooks
is false, which can leave imports unresolved. Update selectFlowLoaderStrategy to
handle this combination explicitly instead of silently returning "none"—either
preserve the expected aliasing behavior or fail/route to a safer strategy—and
add a test in selectFlowLoaderStrategy.test.ts covering caps.hasNativeTypeScript
with caps.hasSyncHooks disabled.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 482e7739-9e0c-491b-9e3f-2ff37b3cfc7d
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (23)
.changeset/support-node-20.md.github/workflows/ci.yml.gitignore.oxlintrc.jsonCONTRIBUTING.mdREADME.mdknip.config.tspackage.jsonscripts/build.tsscripts/buildBinary.tssrc/domains/doctor/checks/nodeVersion.test.tssrc/domains/doctor/checks/nodeVersion.tssrc/domains/runner/loadFlowDefault.tssrc/oxcNode.d.tssrc/shell/resolver/detectRuntimeCapabilities.test.tssrc/shell/resolver/detectRuntimeCapabilities.tssrc/shell/resolver/registerFlowLoader.test.tssrc/shell/resolver/registerFlowLoader.tssrc/shell/resolver/selectFlowLoaderStrategy.test.tssrc/shell/resolver/selectFlowLoaderStrategy.tstest/node20/greeting.tstest/node20/sample.flow.tstest/node20/smoke.mjs
Lower engines.node to >=20.19.0 (the highest real floor across deps: @qawolf/emails wants 20.19, @clack/prompts 20.12, module.register 20.6). On runtimes without native TypeScript (Node 20, Node 22.15-22.17) register the @oxc-node/core ESM loader to transpile and resolve flow modules; Bun and Node 22.18+ keep their existing paths. A Node without native TS and without module.register (< 20.6) fails with a clear unsupported-runtime error, and a failed oxc-addon load reports an actionable message. The doctor node-version check compares full major.minor.patch so Node below the floor fails correctly. @oxc-node/core stays external in both the npm bundle and the compiled binary (unreachable there). Add a runtime-smoke CI matrix (Node 20/22/24 + Bun) that boots the published bundle and drives the real registerFlowLoader against a TypeScript flow whose sibling import exercises .js->.ts aliasing. The @qawolf/* platform packages declare engines >=22.22.0 but are verified to run on Node 20.19; installs on Node 20 print non-fatal EBADENGINE warnings.
Simon Ingeson (smonn)
force-pushed
the
wiz-10992-support-node-20
branch
from
31cc7f3 to
c2e13b2
Compare
Mateus Zitelli (MateusZitelli)
approved these changes
Jul 6, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Overview of Changes
Lowers the
engines.nodefloor to>=20.19.0so the CLI installs in the Node 20 environments. On runtimes without native TypeScript (Node 20, Node 22.15–22.17) flows are loaded through the@oxc-node/coreESM loader, which transpiles and resolves TS at runtime; Bun and Node 22.18+ keep their existing paths.Testing
Verified end-to-end on a real Node 20.19.0 binary; a new
runtime-smokeCI matrix drives the real loader on Node 20/22/24 + Bun.bun run typecheck bun run lint bun run format:check bun run knip bun run test bun run buildChecklist
Note:
@qawolf/*platform packages declareengines.node >=22.22.0; installs on Node 20 print non-fatalEBADENGINEwarnings but the packages are verified to run on 20.19.