feat!: drop Node 18 support and add 24#598
Conversation
WalkthroughWorkflows: CI job matrices updated to Node versions [20.x, 22.x, 24.x] (removed 18.x); several workflow steps renamed/normalized, secret descriptions standardized, Sequence Diagram(s)mermaid Note over CI,Runner Note over Runner,Loader Note over Runner,Loader Pre-merge checks❌ Failed checks (1 inconclusive)
✅ Passed checks (1 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/pages/src/upgrade/pagesUpdater.ts (1)
553-572: Update version check to align with dropped Node 18 support.The
checkNodeVersionfunction at line 564 still explicitly checks for Node 18 (if (version !== 18 && version !== 20)), which is inconsistent with the updatedNODE_ENGINESconstant that no longer includes Node 18 support.Apply this diff to update the version check:
- if (version !== 18 && version !== 20) { + if (version !== 20 && version !== 22 && version !== 24) { console.error( `You are currently using an unsupported node version ${nodeVersion}. Please install node ${NODE_ENGINES}.` );
🧹 Nitpick comments (1)
packages/pages/src/bin/spawn.ts (1)
13-18: Remove or comment out unused nodeVersion calculation.The nodeVersion constant is calculated but never used. This wastes resources by spawning a node process on every invocation. Additionally, the comment incorrectly refers to it as a "function."
Option 1 (preferred): Remove the dead code entirely.
-// Keeping this unused function in case it's needed again -const nodeVersion = Number( - spawnSync("node", ["-v"], { encoding: "utf-8" }) - .stdout.substring(1) - .split(".")[0] -); -Option 2: Comment it out if truly needed for reference.
-// Keeping this unused function in case it's needed again -const nodeVersion = Number( - spawnSync("node", ["-v"], { encoding: "utf-8" }) - .stdout.substring(1) - .split(".")[0] -); +// Commented out - can be restored if Node version checks are needed: +// const nodeVersion = Number( +// spawnSync("node", ["-v"], { encoding: "utf-8" }) +// .stdout.substring(1) +// .split(".")[0] +// );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
.github/workflows/build.yml(1 hunks).github/workflows/jstest.yml(2 hunks).github/workflows/playwright.yml(4 hunks).github/workflows/third_party_notices_check.yml(4 hunks).github/workflows/unit_test.yml(2 hunks)package.json(2 hunks)packages/pages/package.json(2 hunks)packages/pages/src/bin/spawn.ts(1 hunks)packages/pages/src/upgrade/pagesUpdater.ts(1 hunks)playground/locations-site/package.json(2 hunks)playground/multibrand-site/package.json(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: call_playwright / acceptance (windows-latest, 24.x)
- GitHub Check: call_playwright / acceptance (macos-latest, 24.x)
- GitHub Check: call_playwright / acceptance (macos-latest, 22.x)
- GitHub Check: call_playwright / acceptance (macos-latest, 20.x)
- GitHub Check: call_playwright / acceptance (windows-latest, 22.x)
- GitHub Check: call_playwright / acceptance (ubuntu-latest, 22.x)
- GitHub Check: call_playwright / acceptance (ubuntu-latest, 20.x)
- GitHub Check: call_playwright / acceptance (ubuntu-latest, 24.x)
- GitHub Check: call_playwright / acceptance (windows-latest, 20.x)
- GitHub Check: call_unit_test / unit_tests (22.x)
- GitHub Check: call_jstest / jstest
- GitHub Check: call_unit_test / unit_tests (20.x)
- GitHub Check: call_unit_test / unit_tests (24.x)
- GitHub Check: semgrep/ci
🔇 Additional comments (13)
packages/pages/src/upgrade/pagesUpdater.ts (1)
527-528: LGTM! Node engine range updated correctly.The constant now reflects the new supported Node versions (20.2.0+, 22, and 24), aligning with the PR objective to drop Node 18 support.
.github/workflows/unit_test.yml (1)
19-43: LGTM! Test matrix updated to reflect new Node version support.The workflow now tests on Node 20.x, 22.x, and 24.x, correctly dropping Node 18 support as intended.
.github/workflows/third_party_notices_check.yml (2)
15-16: LGTM! Node version updated appropriately.The workflow now uses Node 20.x, aligning with the updated minimum supported version.
25-46: LGTM! Syntax normalization improves consistency.The workflow syntax has been normalized (removing extra space in
run:directives) and quote style standardized..github/workflows/build.yml (1)
19-45: LGTM! Build matrix updated correctly.The build workflow now targets Node 20.x, 22.x, and 24.x, consistent with the updated engine requirements.
package.json (1)
25-27: LGTM! Engine specification updated correctly.The Node engine requirement now enforces the new minimum version (20.2.0) and adds support for Node 24, while dropping Node 18.
playground/locations-site/package.json (1)
14-16: LGTM! Engine specification aligned with project requirements.The playground package now enforces the same Node version requirements as the root package, ensuring consistency across the monorepo.
.github/workflows/playwright.yml (2)
6-7: LGTM! Secret requirement explicit.Adding
required: trueto the secret declaration is a good practice that makes the workflow dependency clear.
15-68: LGTM! Playwright tests updated for new Node versions.The test matrix now includes Node 20.x, 22.x, and 24.x across all operating systems (Windows, Ubuntu, macOS), ensuring comprehensive coverage.
.github/workflows/jstest.yml (2)
5-6: LGTM! Quote style normalized.The description now uses double quotes, consistent with other workflow files in this PR.
21-25: LGTM! JavaScript tests updated to Node 20.The workflow now uses Node 20 for building and testing, aligning with the new minimum supported version.
playground/multibrand-site/package.json (1)
15-15: LGTM! Node version constraint updated correctly.The engines.node update appropriately drops Node 18 support and adds Node 24, aligning with the PR objectives.
packages/pages/package.json (1)
39-39: All dependencies verified compatible with Node 24; change is safe.VitePress 1.5.0 declares a broad Node requirement (>=14.6.0) and depends on Vite, which explicitly requires Node 20.19+ or 22.12+—directly compatible with your engines specification. Rollup 4.27.2, Express 4.18.3, and TypeScript 5.4.2 all support Node 24 through their permissive engine constraints. No breaking incompatibilities identified.
|
lgtm but not sure why call_unit_test / unit_tests (18.x) is trying to run still |
|
unit_tests (18.x) is required in the GitHub settings so it's expecting it to run -- that will need to be updated |
updated |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/pages/src/upgrade/pagesUpdater.ts (1)
552-571: Critical: Update the hardcoded version check to match NODE_ENGINES.The
checkNodeVersion()function still checks for Node 18 and 20 only, but theNODE_ENGINESconstant now supports "^20.6.0 || ^22 || ^24". This inconsistency will cause:
- Node 18 to be incorrectly accepted (should be rejected)
- Node 22 and 24 to be incorrectly rejected (should be accepted)
Apply this diff to fix the version check:
const version = parseInt(nodeVersion.toString().split(".")[0].substring(1)); - if (version !== 18 && version !== 20) { + if (version !== 20 && version !== 22 && version !== 24) { console.error( `You are currently using an unsupported node version ${nodeVersion}. Please install node ${NODE_ENGINES}.` ); }Note: This simplified check only validates the major version. Consider whether the minimum Node 20.6.0 requirement needs additional validation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
package.json(2 hunks)packages/pages/package.json(2 hunks)packages/pages/src/upgrade/pagesUpdater.ts(1 hunks)playground/locations-site/package.json(2 hunks)playground/multibrand-site/package.json(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- playground/multibrand-site/package.json
- packages/pages/package.json
- package.json
- playground/locations-site/package.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: call_playwright / acceptance (macos-latest, 24.x)
- GitHub Check: call_playwright / acceptance (windows-latest, 24.x)
- GitHub Check: call_playwright / acceptance (macos-latest, 22.x)
- GitHub Check: call_playwright / acceptance (macos-latest, 20.x)
- GitHub Check: call_playwright / acceptance (ubuntu-latest, 24.x)
- GitHub Check: call_playwright / acceptance (ubuntu-latest, 20.x)
- GitHub Check: call_unit_test / unit_tests (22.x)
- GitHub Check: call_playwright / acceptance (windows-latest, 20.x)
- GitHub Check: call_playwright / acceptance (windows-latest, 22.x)
- GitHub Check: call_playwright / acceptance (ubuntu-latest, 22.x)
- GitHub Check: call_unit_test / unit_tests (24.x)
- GitHub Check: call_unit_test / unit_tests (20.x)
- GitHub Check: call_jstest / jstest
- GitHub Check: semgrep/ci
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/pages/package.json (1)
39-39: Node version constraint is correct and production-ready.The engines.node update to
"^20.6.0 || ^22 || ^24"properly drops Node 18 support and adds Node 24, which entered Active LTS in October 2025 with support through April 2028. The constraint format is valid.Consider updating
@types/nodefrom^20.11.28to a version that covers Node 24 types (e.g.,^24.0.0) to ensure developers have accurate type definitions when working with Node 24 features.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/pages/package.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: call_playwright / acceptance (ubuntu-latest, 20.x)
- GitHub Check: call_playwright / acceptance (macos-latest, 20.x)
- GitHub Check: call_playwright / acceptance (ubuntu-latest, 22.x)
- GitHub Check: call_playwright / acceptance (macos-latest, 24.x)
- GitHub Check: call_playwright / acceptance (macos-latest, 22.x)
- GitHub Check: call_playwright / acceptance (windows-latest, 20.x)
- GitHub Check: call_playwright / acceptance (windows-latest, 22.x)
- GitHub Check: call_playwright / acceptance (ubuntu-latest, 24.x)
- GitHub Check: call_playwright / acceptance (windows-latest, 24.x)
- GitHub Check: call_jstest / jstest
- GitHub Check: semgrep/ci
3 participants
No description provided.