Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

front: upgrade node to v23, nivo to v0.88 #10442

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from
Open

Conversation

emersion
Copy link
Member

A newer node version is required to make require() work with
modules. This is necessary for a nivo upgrade, otherwise CI fails
with:

 FAIL  src/modules/rollingStock/components/RollingStockEditor/__tests__/RollingStockEditorForm.spec.ts [ src/modules/rollingStock/components/RollingStockEditor/__tests__/RollingStockEditorForm.spec.ts ]
Error: require() of ES Module /app/node_modules/d3-interpolate/src/index.js from /app/node_modules/@nivo/core/dist/nivo-core.cjs.js not supported.
Instead change the require of index.js in /app/node_modules/@nivo/core/dist/nivo-core.cjs.js to a dynamic import() which is available in all CommonJS modules.
 ❯ Object.<anonymous> node_modules/@nivo/core/dist/nivo-core.cjs.js:1:164

See also: #10256

A newer node version is required [1] to make require() work with
modules. This is necessary for a nivo upgrade, otherwise CI fails
with:

     FAIL  src/modules/rollingStock/components/RollingStockEditor/__tests__/RollingStockEditorForm.spec.ts [ src/modules/rollingStock/components/RollingStockEditor/__tests__/RollingStockEditorForm.spec.ts ]
    Error: require() of ES Module /app/node_modules/d3-interpolate/src/index.js from /app/node_modules/@nivo/core/dist/nivo-core.cjs.js not supported.
    Instead change the require of index.js in /app/node_modules/@nivo/core/dist/nivo-core.cjs.js to a dynamic import() which is available in all CommonJS modules.
     ❯ Object.<anonymous> node_modules/@nivo/core/dist/nivo-core.cjs.js:1:164

[1]: plouc/nivo#2310 (comment)

Signed-off-by: Simon Ser <[email protected]>
@emersion emersion requested a review from a team as a code owner January 20, 2025 13:13
@github-actions github-actions bot added the area:front Work on Standard OSRD Interface modules label Jan 20, 2025
@emersion emersion self-assigned this Jan 20, 2025
Copy link
Contributor

@Yohh Yohh left a comment

Choose a reason for hiding this comment

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

you should modify flake.nixtoo:

 fixedNode = pkgs.nodejs_23;

@emersion
Copy link
Member Author

emersion commented Jan 21, 2025

you should modify flake.nix too

Sorry, I can't do that. Updating flake.nix isn't enough AFAIU, the lock file also needs to be updated together, and I don't have/want the nix tool installed locally.

This upgrade is just about CI and Docker, and e.g. I continue to use my distro's node package: we don't need everybody to use the same version. Feel free to submit a separate PR to upgrade nix-related stuff.

Copy link
Contributor

@SharglutDev SharglutDev left a comment

Choose a reason for hiding this comment

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

Lgtm, thank you !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:front Work on Standard OSRD Interface modules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants