Skip to content

test: add Node 20 to node-version matrix (expected red until Vite+ supports it)#84

Merged
fengmk2 merged 4 commits into
mainfrom
add-node-20-matrix
Jun 18, 2026
Merged

test: add Node 20 to node-version matrix (expected red until Vite+ supports it)#84
fengmk2 merged 4 commits into
mainfrom
add-node-20-matrix

Conversation

@fengmk2

@fengmk2 fengmk2 commented Jun 18, 2026

Copy link
Copy Markdown
Member

Adds Node 20 to the test-node-version matrix. The job now runs vp install, which enforces Vite+'s Node engine range (^22.18.0 || >=24.11.0), so Node 20 fails at the engine gate (same failure as rolldown CI).

This is intentional: Node 20 stays red until Vite+ adds Node 20 support, at which point the same matrix turns green with no test changes. lts, 22, and 24 install cleanly and pass.

Cover Node.js 20 in the test-node-version job alongside lts, 22, and 24.
Copilot AI review requested due to automatic review settings June 18, 2026 02:59
@fengmk2 fengmk2 self-assigned this Jun 18, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Expands the CI coverage in .github/workflows/test.yml by adding Node.js 20 to the existing test-node-version matrix, ensuring the action is validated across more supported Node versions.

Changes:

  • Added Node.js 20 to the test-node-version matrix (["lts", "20", "22", "24"]).
  • Kept the existing “Verify Node.js version” logic unchanged (it already treats non-lts versions generically).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Vite+ requires Node ^22.18.0 || >=24.11.0, so Node 20 is unsupported. A
happy-path matrix entry for 20 passed vacuously because the engine gate is
only enforced by workload commands like vp install, not by vp env use or
vp --version. Replace it with a negative test that runs vp install under
Node 20 and asserts it fails with the incompatibility error, reproducing
the rolldown CI failure.
@fengmk2 fengmk2 changed the title test: add node 20 to node-version matrix test: assert Vite+ rejects Node 20 (negative test) Jun 18, 2026
…+ supports it)

Run vp install in the matrix so Node 20 throws at Vite+'s engine gate
(requires ^22.18.0 || >=24.11.0). Node 20 is kept red on purpose until
Vite+ adds Node 20 support; drop the separate negative-test job.
Copilot AI review requested due to automatic review settings June 18, 2026 03:25
@fengmk2 fengmk2 changed the title test: assert Vite+ rejects Node 20 (negative test) test: add Node 20 to node-version matrix (expected red until Vite+ supports it) Jun 18, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comment thread .github/workflows/test.yml
Vite+'s bundled pnpm 11 requires node:sqlite (Node >= 22.5) and crashes on
Node 20. Pin the test project to pnpm@10.34.3 so vp install runs the whole
node-version matrix, including Node 20.
@fengmk2 fengmk2 merged commit a268c9c into main Jun 18, 2026
34 checks passed
@fengmk2 fengmk2 deleted the add-node-20-matrix branch June 18, 2026 05:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants