-
Notifications
You must be signed in to change notification settings - Fork 3
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
Migrate to node 20 #228
Migrate to node 20 #228
Conversation
Coverage after merging chore/migrate-node-20 into main
Coverage Report
|
- @opendatasoft/[email protected] - @opendatasoft/[email protected] - @opendatasoft/[email protected]
Coverage after merging chore/migrate-node-20 into main
Coverage Report
|
Coverage after merging chore/migrate-node-20 into main
Coverage Report
|
2ed6205
to
c583175
Compare
fetch is native in node >= 18
Coverage after merging chore/migrate-node-20 into main
Coverage Report
|
1 similar comment
Coverage after merging chore/migrate-node-20 into main
Coverage Report
|
Coverage after merging chore/migrate-node-20 into main
Coverage Report
|
1 similar comment
Coverage after merging chore/migrate-node-20 into main
Coverage Report
|
Coverage after merging chore/migrate-node-20 into main
Coverage Report
|
1 similar comment
Coverage after merging chore/migrate-node-20 into main
Coverage Report
|
4ac7031
to
0ec17f4
Compare
In order to work with node 20
0ec17f4
to
c751c10
Compare
Coverage after merging chore/migrate-node-20 into main
Coverage Report
|
1 similar comment
Coverage after merging chore/migrate-node-20 into main
Coverage Report
|
Coverage after merging chore/migrate-node-20 into main
Coverage Report
|
1 similar comment
Coverage after merging chore/migrate-node-20 into main
Coverage Report
|
This reverts commit 28487e2.
Coverage after merging chore/migrate-node-20 into main
Coverage Report
|
877f96b
to
9122e5f
Compare
Coverage after merging chore/migrate-node-20 into main
Coverage Report
|
1 similar comment
Coverage after merging chore/migrate-node-20 into main
Coverage Report
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GG for the migration 👏
I left some comments to have explanation on specific topics, but nothing blocking
Coverage after merging chore/migrate-node-20 into main
Coverage Report
|
1 similar comment
Coverage after merging chore/migrate-node-20 into main
Coverage Report
|
Coverage after merging chore/migrate-node-20 into main
Coverage Report
|
1 similar comment
Coverage after merging chore/migrate-node-20 into main
Coverage Report
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A bit of clean up to do around the package.json and yml files are we should be good 😄
@@ -53,7 +51,7 @@ | |||
"trailingComma": "es5" | |||
}, | |||
"dependencies": { | |||
"@opendatasoft/visualizations": "0.21.1", | |||
"@opendatasoft/visualizations": "^0.21.1-beta.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please keep to an exact version
"@opendatasoft/visualizations": "^0.21.1-beta.0", | |
"@opendatasoft/visualizations": "0.21.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did the update here
f76087d
Coverage after merging chore/migrate-node-20 into main
Coverage Report
|
1 similar comment
Coverage after merging chore/migrate-node-20 into main
Coverage Report
|
15cf2de
to
33d08b9
Compare
Coverage after merging chore/migrate-node-20 into main
Coverage Report
|
1 similar comment
Coverage after merging chore/migrate-node-20 into main
Coverage Report
|
I added an |
Coverage after merging chore/migrate-node-20 into main
Coverage Report
|
1 similar comment
Coverage after merging chore/migrate-node-20 into main
Coverage Report
|
c34c893
to
f76087d
Compare
Coverage after merging chore/migrate-node-20 into main
Coverage Report
|
1 similar comment
Coverage after merging chore/migrate-node-20 into main
Coverage Report
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine, I just added a commit to restore the good packages versions. Since your started this branch, we had some releases
@etienneburdet |
Summary
The goal for this PR is to upgrade the engines requirements for node.
Changes
fetch
, since we don't plan to support node =< 18 andfetch
is part of the API since 18 on (and should be stable in 21…).I also had to upgrade lerna and storybook, because old version weren't fully compatible with new node. This makes for a few breaking changes:
lerna bootstrap, add, link
👉 npm workspaces handle all of that with npm install. Lerna (Nx) is still use for running tasks in parallel (namely watch) and manage versions.vite
. It paves the way for actually using vite everywhere, why not migrating to SvelteKit, moving the storybook back to Svelte etc. But for now this is just a minimal port.Else I just upgrade all references to nodes >=18 (current LTS) and checked it works with node 20.
Changelog
All packages are now supposed to run on node 18 or 20 (prefered). Support for node 14 and 16 is dropped.