Bump node to v24#6704
Conversation
WalkthroughNode.js version references updated from 20 to 24 across CI/CD workflows and Copilot docs; Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
⏰ 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). (2)
🔇 Additional comments (1)
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: 2
🧹 Nitpick comments (2)
.github/workflows/copilot-setup-steps.yml (1)
39-39: Consider normalizing Node version specification.While
node-version: "24"works correctly, other workflows in this PR consistently use"24.x"format. For consistency across the codebase, consider updating this to match.- name: Set up Node.js uses: actions/setup-node@v4 with: - node-version: "24" + node-version: "24.x" cache: "npm".github/workflows/npm-publish.yml (1)
20-20: Consider normalizing Node version specification.While
node-version: 24works correctly with setup-node@v3, other workflows consistently use"24.x"format. For consistency, consider updating this to match the pattern used in other workflows (e.g., ci.yml, deploy-website.yml).- uses: actions/setup-node@v3 with: - node-version: 24 + node-version: "24.x" registry-url: https://registry.npmjs.org/
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (9)
.github/copilot.md(1 hunks).github/workflows/ci.yml(1 hunks).github/workflows/copilot-setup-steps.yml(1 hunks).github/workflows/deploy-staging.yml(1 hunks).github/workflows/deploy-website.yml(1 hunks).github/workflows/npm-publish.yml(1 hunks).github/workflows/sync-playwright-versions.yml(1 hunks).github/workflows/update-vr-snapshots.yml(1 hunks)package.json(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-11-25T01:23:14.964Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T01:23:14.964Z
Learning: ALWAYS unset `NODE_ENV` environment variable when working in this project
Applied to files:
.github/copilot.md
📚 Learning: 2025-11-25T01:22:48.279Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: DEVELOPING.md:0-0
Timestamp: 2025-11-25T01:22:48.279Z
Learning: Applies to test-vr/__snapshots__/** : Commit new or updated visual regression test snapshots from the `test-vr/__snapshots__` directory to the repository
Applied to files:
.github/workflows/update-vr-snapshots.yml
⏰ 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). (2)
- GitHub Check: Build, Test, Pack
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (6)
.github/workflows/sync-playwright-versions.yml (1)
34-34: LGTM!Node.js version update is consistent with other workflows in this PR.
.github/workflows/update-vr-snapshots.yml (1)
19-19: LGTM!Node.js environment variable updated correctly and used consistently throughout the workflow.
.github/workflows/deploy-website.yml (1)
13-13: LGTM!Node.js environment variable update is consistent with other workflows.
.github/copilot.md (1)
22-22: LGTM!Documentation accurately reflects Node.js version requirement update.
.github/workflows/ci.yml (1)
18-18: LGTM!Node.js environment variable correctly updated and consistently applied throughout the workflow.
package.json (1)
130-130: @types/[email protected] is the correct version specifier for Node.js 24.The web search confirms that @types/node v24 is the proper TypeScript typings package aligned with Node.js 24. The 24.x.x version specification matches the Node.js 24 runtime and will include the appropriate Node.js 24 APIs in type definitions. No compatibility issues were found.
|
|
||
| env: | ||
| NODE_VERSION: 20.x | ||
| NODE_VERSION: 24.x |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
actions/setup-node@v4 Node.js 24.x support compatibility
💡 Result:
Short answer: use a newer setup-node release. actions/setup-node@v4 was not updated to run on Node 24 as its action runtime; GitHub’s migration to Node 24 is handled at the runner level and newer setup-node releases (v5+/v6) explicitly mention Node‑24 compatibility. For reliable Node 24 usage prefer the latest setup-node (or v6) and ensure your runners support Node 24 (you can force-test Node 24 on runners with FORCE_JAVASCRIPT_ACTIONS_TO_NODE24=true). [1][2]
Sources:
- actions/setup-node repository (action releases / usage notes). [1]
- GitHub changelog: deprecation/migration to Node 24 on Actions runners (how to test/opt out). [2]
🌐 Web query:
Node.js 24 breaking changes npm compatibility
💡 Result:
Short summary — Node.js 24 (Current, released 2025) includes some breaking/behavior changes you should check and upgrades npm compatibility:
Key compatibility facts
- Node 24 ships bundled with npm 11.0.x. npm 11’s stated Node support range is ^20.17.0 || >=22.9.0 (npm 11 is the shipped client for Node 24). [1][2]
npm-related breaking/behavior changes to watch
- npm 11 removes the npm hook command. [1]
- --ignore-scripts now applies to all lifecycle scripts (including prepare). This can change install/build behavior for projects that relied on prepare or other lifecycle scripts being skipped. [1][2]
- npm init adds a new type prompt and may produce a different sorted package.json layout. [1][2]
- npm no longer falls back to the old audit endpoint if the bulk advisory request fails (affects audit behavior). [1]
Node.js 24 runtime changes that affect packages
- Undici upgraded to v7 (HTTP client) — some Undici v7 breaking changes may affect libraries or code using fetch/undici directly. [1][3]
- V8 → 13.6 (new JS features); AsyncLocalStorage internals changed (AsyncContextFrame), URLPattern is global now, and some legacy APIs were removed/deprecated (legacy URL parser, certain tls/zlib usages). These runtime changes can surface as compatibility issues in dependencies or native modules. [3][4]
Practical upgrade checklist
- Rebuild native modules (npm rebuild / reinstall native deps). [2]
- Upgrade CI images / engine fields to node 24 where appropriate and run full test suite.
- If you depend on lifecycle scripts, review prepare/build script behavior because of npm 11’s --ignore-scripts change.
- Audit any direct uses of Undici/fetch, legacy URL/tls/zlib APIs, and code relying on previous AsyncLocalStorage behavior.
Sources
- Node.js / OpenJS Foundation release notes and highlights for v24. [3]
- NodeSource / community summaries noting npm 11, Undici 7 and npm changes. [1]
- Blog/upgrade guides summarizing npm 11 behavior and upgrade steps. [2][4]
Upgrade to actions/setup-node@v5 or v6 required; actions/setup-node@v4 does not officially support Node.js 24.x.
The original review incorrectly approved this change. Web verification reveals critical compatibility issues:
-
actions/setup-node@v4 is not officially updated for Node.js 24.x. GitHub's migration to Node 24 occurs at the runner level, but newer action versions (v5+/v6) explicitly document Node 24 support. Using v4 with Node 24 is not guaranteed to work reliably.
-
Node.js 24 includes breaking changes affecting CI/CD:
- npm 11 (bundled with Node 24) removes the
npm hookcommand and changes--ignore-scriptsbehavior to apply to all lifecycle scripts (includingprepare), which can alter install/build behavior. - Undici upgraded to v7 with potential breaking changes for HTTP/fetch operations.
- V8 13.6 and deprecated legacy APIs (URL parser, tls/zlib) may affect dependencies or native modules.
- npm 11 (bundled with Node 24) removes the
-
Workflow action version must be upgraded: Change line 27–30 from
actions/setup-node@v4toactions/setup-node@v6(or v5) to ensure explicit Node.js 24 compatibility. -
Validate npm 11 behavior: The workflow's npm install and build steps should be tested to confirm no lifecycle script breakage or dependency resolution issues.
🤖 Prompt for AI Agents
.github/workflows/deploy-staging.yml around line 11 (and change block lines
~27–30): the workflow sets NODE_VERSION: 24.x but still uses
actions/setup-node@v4 which doesn’t officially support Node 24; update the setup
action to actions/setup-node@v6 (or @v5) by replacing the v4 reference, keep
NODE_VERSION: 24.x, and run CI locally/once in staging to validate npm 11
behavior (install/build/lifecycle scripts) and confirm no failures from
undici/V8/native deps; if npm lifecycle or native module issues appear, pin a
compatible Node version or add explicit npm flags/workarounds as needed.
There was a problem hiding this comment.
actions/setup-node v4 looks like it works just fine with node24. Where does it say it's incompatible?
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
actions/setup-node v4 Node.js 24 compatibility official documentation
💡 Result:
Short answer: actions/setup-node v4 can install/use Node.js 24 for your workflow jobs (you can set node-version: 24), but the action runtime itself was not upgraded to run on Node 24 until later releases (v5/v6). See the official action README for supported node-version syntax and examples, and the releases/changelog showing the action runtime upgrade to Node 24 and the runner compatibility note. [1][2][3]
Sources
- actions/setup-node (README) — supported node-version examples and usage (shows using 24). [1]
- actions/setup-node releases — notes about upgrading the action runtime to Node24 in later releases. [2]
- GitHub Changelog — runner / Node24 compatibility and migration guidance. [3]
@PavelVanecek You're absolutely right, and I apologize for the confusion in my previous comment.
actions/setup-node@v4 works perfectly fine with Node.js 24.x for your use case. The action can successfully install and configure Node.js 24 when you specify node-version: 24.x.
The confusion arose from conflating two different things:
- The Node version the action installs for your workflow (which v4 supports, including Node 24) ✅
- The Node version the action's own runtime uses internally (which was upgraded to Node 24 in v5/v6, but doesn't affect your workflow)
For your workflow's purposes—installing and using Node 24.x to build your project—actions/setup-node@v4 with NODE_VERSION: 24.x is completely valid and will work as expected. There's no need to upgrade the action version.
Your current configuration is fine. Sorry for the noise! 🐰
🧠 Learnings used
Learnt from: CR
Repo: recharts/recharts PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T01:23:14.964Z
Learning: ALWAYS unset `NODE_ENV` environment variable when working in this project
|
Couple tests are failing because the rounding has changed. |
## Description Round numbers to 4 decimal places before using them as paths. Also updates all relevant tests. ## Related Issue #6704 ## Motivation and Context Something between node22 and node24 changed how floating numbers are calculated and that made our numbers differ somewhere at 10th decimal place. Since we're dealing with pixels we don't care about anything after the decimal dot so I figured we may just as well round it. I tried rounding to 0 decimal places but that creates ton of visual diff (somehow), so I made it 4 decimal places. The tests are now passing at node22, node24, and 25 the same. I have considered rounding in the test helpers instead but that's an uphill battle. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added automatic rounding of numeric coordinates to improve consistency and predictability in chart rendering. * **Bug Fixes** * Reduced floating-point precision artifacts in visualization outputs, resulting in cleaner and more consistent display of pie charts, bar charts, radar charts, and other diagram types. <sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
@copilot rebase on main |
|
@PavelVanecek I've opened a new pull request, #6709, to work on those changes. Once the pull request is ready, I'll request review from you. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/npm-publish.yml (1)
17-21: Align Node.js version format and update setup-node action to v4.Two maintenance concerns:
- Version format inconsistency: This file uses
24(unquoted, no .x) whilesync-playwright-versions.ymluses'24.x'. Consider standardizing across all workflows to'24.x'for consistency.- setup-node action version: Line 18 uses
actions/setup-node@v3, which is older than the@v4used in other workflows in this PR. For npm publish (a security-critical path), upgrading to@v4is recommended for access to latest bug fixes and security patches.Apply this diff to align with best practices:
- uses: actions/checkout@v3 - - uses: actions/setup-node@v3 + - uses: actions/setup-node@v4 with: - node-version: 24 + node-version: '24.x' registry-url: https://registry.npmjs.org/
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (10)
.github/copilot.md(1 hunks).github/workflows/ci.yml(1 hunks).github/workflows/copilot-setup-steps.yml(1 hunks).github/workflows/deploy-staging.yml(1 hunks).github/workflows/deploy-website.yml(1 hunks).github/workflows/npm-publish.yml(1 hunks).github/workflows/sync-playwright-versions.yml(1 hunks).github/workflows/update-vr-snapshots.yml(1 hunks).nvmrc(1 hunks)package.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .nvmrc
🚧 Files skipped from review as they are similar to previous changes (7)
- .github/workflows/deploy-website.yml
- package.json
- .github/workflows/update-vr-snapshots.yml
- .github/workflows/ci.yml
- .github/copilot.md
- .github/workflows/deploy-staging.yml
- .github/workflows/copilot-setup-steps.yml
⏰ 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). (2)
- GitHub Check: Build, Test, Pack
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
.github/workflows/sync-playwright-versions.yml (1)
31-35: Node.js version update is straightforward and consistent.The version bump to Node.js 24.x is properly formatted and aligns with actions/setup-node@v4 expectations. The workflow commands (npm ci, npm run, and node script execution) are compatible with Node.js 24.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6704 +/- ##
=======================================
Coverage 94.03% 94.03%
=======================================
Files 500 500
Lines 42657 42657
Branches 4901 4901
=======================================
Hits 40111 40111
Misses 2541 2541
Partials 5 5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Bundle ReportBundle size has no change ✅ |
|
All tests are passing now. @ckifer thoughts? This should be a dev-only change. |
|
AHA nvm, the workflow file needs one more fix |
|
I'm going to merge this to unblock other PRs - I have bumped myself into a corner. |
Description
Storybook v10 requires node22+, and node 24+ also allows running typescript directly without a compilation step which is nice and makes our lives easier so let's bump up to 24.
Summary by CodeRabbit
Chores
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.