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

Migrate to node 20 #228

Merged
merged 22 commits into from
Apr 12, 2024
Merged

Migrate to node 20 #228

merged 22 commits into from
Apr 12, 2024

Conversation

etienneburdet
Copy link
Contributor

@etienneburdet etienneburdet commented Mar 8, 2024

Summary

The goal for this PR is to upgrade the engines requirements for node.

Changes

  • Disabled rollup visualizer plugin to avoid migrating the full stack.
  • Removed the check for fetch, since we don't plan to support node =< 18 and fetch is part of the API since 18 on (and should be stable in 21…).
  • Replace @mapbox/geo-viewport with @placemarkio/geo-viewport
  • Remove run-p/npm-run-all in favor of lerna/nx
  • Upgrade to ChartJS 4 because of some TS problems

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:

  • No more 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.
  • Storybook now runs on 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.

Copy link

github-actions bot commented Mar 8, 2024

Coverage after merging chore/migrate-node-20 into main

95.21%

Coverage Report
FileBranchesFuncsLinesUncovered Lines
src
   index.ts100%100%100%
src/client
   error.ts100%100%100%
   index.ts75.32%100%96.88%124, 13, 146, 148, 148–149, 15, 15, 151, 162, 169, 169, 17, 17, 171, 176, 179, 182, 184, 52, 82
   types.ts100%100%100%
src/odsql
   clauses.ts71.43%80%90.91%14, 32, 42
   index.ts83.72%95.65%94.12%111, 140, 25, 28, 56–57, 57, 57–58, 68, 78–79

Copy link

github-actions bot commented Mar 8, 2024

Coverage after merging chore/migrate-node-20 into main

95.21%

Coverage Report
FileBranchesFuncsLinesUncovered Lines
src
   index.ts100%100%100%
src/client
   error.ts100%100%100%
   index.ts75.32%100%96.88%124, 13, 146, 148, 148–149, 15, 15, 151, 162, 169, 169, 17, 17, 171, 176, 179, 182, 184, 52, 82
   types.ts100%100%100%
src/odsql
   clauses.ts71.43%80%90.91%14, 32, 42
   index.ts83.72%95.65%94.12%111, 140, 25, 28, 56–57, 57, 57–58, 68, 78–79

@etienneburdet etienneburdet marked this pull request as ready for review March 11, 2024 08:54
Copy link

Coverage after merging chore/migrate-node-20 into main

95.21%

Coverage Report
FileBranchesFuncsLinesUncovered Lines
src
   index.ts100%100%100%
src/client
   error.ts100%100%100%
   index.ts75.32%100%96.88%124, 13, 146, 148, 148–149, 15, 15, 151, 162, 169, 169, 17, 17, 171, 176, 179, 182, 184, 52, 82
   types.ts100%100%100%
src/odsql
   clauses.ts71.43%80%90.91%14, 32, 42
   index.ts83.72%95.65%94.12%111, 140, 25, 28, 56–57, 57, 57–58, 68, 78–79

@etienneburdet etienneburdet marked this pull request as draft March 11, 2024 08:55
@etienneburdet etienneburdet force-pushed the chore/migrate-node-20 branch from 2ed6205 to c583175 Compare March 11, 2024 08:56
@etienneburdet etienneburdet marked this pull request as ready for review March 11, 2024 08:56
fetch is native in node >= 18
Copy link

Coverage after merging chore/migrate-node-20 into main

94.61%

Coverage Report
FileBranchesFuncsLinesUncovered Lines
src
   index.ts100%100%100%
src/client
   error.ts100%100%100%
   index.ts74.03%100%95.31%102–103, 124, 13, 146, 148, 148–149, 15, 15, 151, 162, 169, 169, 17, 17, 171, 176, 179, 182, 184, 52, 82
   types.ts100%100%100%
src/odsql
   clauses.ts71.43%80%90.91%14, 32, 42
   index.ts83.72%95.65%94.12%111, 140, 25, 28, 56–57, 57, 57–58, 68, 78–79

1 similar comment
Copy link

Coverage after merging chore/migrate-node-20 into main

94.61%

Coverage Report
FileBranchesFuncsLinesUncovered Lines
src
   index.ts100%100%100%
src/client
   error.ts100%100%100%
   index.ts74.03%100%95.31%102–103, 124, 13, 146, 148, 148–149, 15, 15, 151, 162, 169, 169, 17, 17, 171, 176, 179, 182, 184, 52, 82
   types.ts100%100%100%
src/odsql
   clauses.ts71.43%80%90.91%14, 32, 42
   index.ts83.72%95.65%94.12%111, 140, 25, 28, 56–57, 57, 57–58, 68, 78–79

Copy link

Coverage after merging chore/migrate-node-20 into main

94.61%

Coverage Report
FileBranchesFuncsLinesUncovered Lines
src
   index.ts100%100%100%
src/client
   error.ts100%100%100%
   index.ts74.03%100%95.31%102–103, 124, 13, 146, 148, 148–149, 15, 15, 151, 162, 169, 169, 17, 17, 171, 176, 179, 182, 184, 52, 82
   types.ts100%100%100%
src/odsql
   clauses.ts71.43%80%90.91%14, 32, 42
   index.ts83.72%95.65%94.12%111, 140, 25, 28, 56–57, 57, 57–58, 68, 78–79

1 similar comment
Copy link

Coverage after merging chore/migrate-node-20 into main

94.61%

Coverage Report
FileBranchesFuncsLinesUncovered Lines
src
   index.ts100%100%100%
src/client
   error.ts100%100%100%
   index.ts74.03%100%95.31%102–103, 124, 13, 146, 148, 148–149, 15, 15, 151, 162, 169, 169, 17, 17, 171, 176, 179, 182, 184, 52, 82
   types.ts100%100%100%
src/odsql
   clauses.ts71.43%80%90.91%14, 32, 42
   index.ts83.72%95.65%94.12%111, 140, 25, 28, 56–57, 57, 57–58, 68, 78–79

Copy link

Coverage after merging chore/migrate-node-20 into main

94.61%

Coverage Report
FileBranchesFuncsLinesUncovered Lines
src
   index.ts100%100%100%
src/client
   error.ts100%100%100%
   index.ts74.03%100%95.31%102–103, 124, 13, 146, 148, 148–149, 15, 15, 151, 162, 169, 169, 17, 17, 171, 176, 179, 182, 184, 52, 82
   types.ts100%100%100%
src/odsql
   clauses.ts71.43%80%90.91%14, 32, 42
   index.ts83.72%95.65%94.12%111, 140, 25, 28, 56–57, 57, 57–58, 68, 78–79

1 similar comment
Copy link

Coverage after merging chore/migrate-node-20 into main

94.61%

Coverage Report
FileBranchesFuncsLinesUncovered Lines
src
   index.ts100%100%100%
src/client
   error.ts100%100%100%
   index.ts74.03%100%95.31%102–103, 124, 13, 146, 148, 148–149, 15, 15, 151, 162, 169, 169, 17, 17, 171, 176, 179, 182, 184, 52, 82
   types.ts100%100%100%
src/odsql
   clauses.ts71.43%80%90.91%14, 32, 42
   index.ts83.72%95.65%94.12%111, 140, 25, 28, 56–57, 57, 57–58, 68, 78–79

@etienneburdet etienneburdet force-pushed the chore/migrate-node-20 branch from 4ac7031 to 0ec17f4 Compare March 12, 2024 10:38
In order to work with node 20
@etienneburdet etienneburdet force-pushed the chore/migrate-node-20 branch from 0ec17f4 to c751c10 Compare March 12, 2024 10:42
Copy link

Coverage after merging chore/migrate-node-20 into main

94.61%

Coverage Report
FileBranchesFuncsLinesUncovered Lines
src
   index.ts100%100%100%
src/client
   error.ts100%100%100%
   index.ts74.03%100%95.31%102–103, 124, 13, 146, 148, 148–149, 15, 15, 151, 162, 169, 169, 17, 17, 171, 176, 179, 182, 184, 52, 82
   types.ts100%100%100%
src/odsql
   clauses.ts71.43%80%90.91%14, 32, 42
   index.ts83.72%95.65%94.12%111, 140, 25, 28, 56–57, 57, 57–58, 68, 78–79

1 similar comment
Copy link

Coverage after merging chore/migrate-node-20 into main

94.61%

Coverage Report
FileBranchesFuncsLinesUncovered Lines
src
   index.ts100%100%100%
src/client
   error.ts100%100%100%
   index.ts74.03%100%95.31%102–103, 124, 13, 146, 148, 148–149, 15, 15, 151, 162, 169, 169, 17, 17, 171, 176, 179, 182, 184, 52, 82
   types.ts100%100%100%
src/odsql
   clauses.ts71.43%80%90.91%14, 32, 42
   index.ts83.72%95.65%94.12%111, 140, 25, 28, 56–57, 57, 57–58, 68, 78–79

Copy link

Coverage after merging chore/migrate-node-20 into main

94.61%

Coverage Report
FileBranchesFuncsLinesUncovered Lines
src
   index.ts100%100%100%
src/client
   error.ts100%100%100%
   index.ts74.03%100%95.31%102–103, 124, 13, 146, 148, 148–149, 15, 15, 151, 162, 169, 169, 17, 17, 171, 176, 179, 182, 184, 52, 82
   types.ts100%100%100%
src/odsql
   clauses.ts71.43%80%90.91%14, 32, 42
   index.ts83.72%95.65%94.12%111, 140, 25, 28, 56–57, 57, 57–58, 68, 78–79

1 similar comment
Copy link

Coverage after merging chore/migrate-node-20 into main

94.61%

Coverage Report
FileBranchesFuncsLinesUncovered Lines
src
   index.ts100%100%100%
src/client
   error.ts100%100%100%
   index.ts74.03%100%95.31%102–103, 124, 13, 146, 148, 148–149, 15, 15, 151, 162, 169, 169, 17, 17, 171, 176, 179, 182, 184, 52, 82
   types.ts100%100%100%
src/odsql
   clauses.ts71.43%80%90.91%14, 32, 42
   index.ts83.72%95.65%94.12%111, 140, 25, 28, 56–57, 57, 57–58, 68, 78–79

Copy link

Coverage after merging chore/migrate-node-20 into main

94.61%

Coverage Report
FileBranchesFuncsLinesUncovered Lines
src
   index.ts100%100%100%
src/client
   error.ts100%100%100%
   index.ts74.03%100%95.31%102–103, 124, 13, 146, 148, 148–149, 15, 15, 151, 162, 169, 169, 17, 17, 171, 176, 179, 182, 184, 52, 82
   types.ts100%100%100%
src/odsql
   clauses.ts71.43%80%90.91%14, 32, 42
   index.ts83.72%95.65%94.12%111, 140, 25, 28, 56–57, 57, 57–58, 68, 78–79

@etienneburdet etienneburdet force-pushed the chore/migrate-node-20 branch from 877f96b to 9122e5f Compare March 19, 2024 09:21
Copy link

Coverage after merging chore/migrate-node-20 into main

94.61%

Coverage Report
FileBranchesFuncsLinesUncovered Lines
src
   index.ts100%100%100%
src/client
   error.ts100%100%100%
   index.ts74.03%100%95.31%102–103, 124, 13, 146, 148, 148–149, 15, 15, 151, 162, 169, 169, 17, 17, 171, 176, 179, 182, 184, 52, 82
   types.ts100%100%100%
src/odsql
   clauses.ts71.43%80%90.91%14, 32, 42
   index.ts83.72%95.65%94.12%111, 140, 25, 28, 56–57, 57, 57–58, 68, 78–79

1 similar comment
Copy link

Coverage after merging chore/migrate-node-20 into main

94.61%

Coverage Report
FileBranchesFuncsLinesUncovered Lines
src
   index.ts100%100%100%
src/client
   error.ts100%100%100%
   index.ts74.03%100%95.31%102–103, 124, 13, 146, 148, 148–149, 15, 15, 151, 162, 169, 169, 17, 17, 171, 176, 179, 182, 184, 52, 82
   types.ts100%100%100%
src/odsql
   clauses.ts71.43%80%90.91%14, 32, 42
   index.ts83.72%95.65%94.12%111, 140, 25, 28, 56–57, 57, 57–58, 68, 78–79

Copy link
Contributor

@KevinFabre-ods KevinFabre-ods left a 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

.github/workflows/visualizations.yml Outdated Show resolved Hide resolved
.github/workflows/visualizations.yml Show resolved Hide resolved
packages/api-client/package-lock.json Show resolved Hide resolved
packages/visualizations-react/.storybook/main.js Outdated Show resolved Hide resolved
packages/visualizations-react/.storybook/main.js Outdated Show resolved Hide resolved
packages/visualizations/package.json Show resolved Hide resolved
packages/visualizations/tsconfig.json Outdated Show resolved Hide resolved
Copy link

Coverage after merging chore/migrate-node-20 into main

94.61%

Coverage Report
FileBranchesFuncsLinesUncovered Lines
src
   index.ts100%100%100%
src/client
   error.ts100%100%100%
   index.ts74.03%100%95.31%102–103, 124, 13, 146, 148, 148–149, 15, 15, 151, 162, 169, 169, 17, 17, 171, 176, 179, 182, 184, 52, 82
   types.ts100%100%100%
src/odsql
   clauses.ts71.43%80%90.91%14, 32, 42
   index.ts83.72%95.65%94.12%111, 140, 25, 28, 56–57, 57, 57–58, 68, 78–79

1 similar comment
Copy link

Coverage after merging chore/migrate-node-20 into main

94.61%

Coverage Report
FileBranchesFuncsLinesUncovered Lines
src
   index.ts100%100%100%
src/client
   error.ts100%100%100%
   index.ts74.03%100%95.31%102–103, 124, 13, 146, 148, 148–149, 15, 15, 151, 162, 169, 169, 17, 17, 171, 176, 179, 182, 184, 52, 82
   types.ts100%100%100%
src/odsql
   clauses.ts71.43%80%90.91%14, 32, 42
   index.ts83.72%95.65%94.12%111, 140, 25, 28, 56–57, 57, 57–58, 68, 78–79

Copy link

Coverage after merging chore/migrate-node-20 into main

94.64%

Coverage Report
FileBranchesFuncsLinesUncovered Lines
src
   index.ts100%100%100%
src/client
   error.ts100%100%100%
   index.ts74.03%100%95.31%102–103, 124, 13, 146, 148, 148–149, 15, 15, 151, 162, 169, 169, 17, 17, 171, 176, 179, 182, 184, 52, 82
   types.ts100%100%100%
src/odsql
   clauses.ts71.43%80%90.91%14, 32, 42
   index.ts83.72%95.74%94.19%111, 146, 25, 28, 56–57, 57, 57–58, 68, 78–79

1 similar comment
Copy link

Coverage after merging chore/migrate-node-20 into main

94.64%

Coverage Report
FileBranchesFuncsLinesUncovered Lines
src
   index.ts100%100%100%
src/client
   error.ts100%100%100%
   index.ts74.03%100%95.31%102–103, 124, 13, 146, 148, 148–149, 15, 15, 151, 162, 169, 169, 17, 17, 171, 176, 179, 182, 184, 52, 82
   types.ts100%100%100%
src/odsql
   clauses.ts71.43%80%90.91%14, 32, 42
   index.ts83.72%95.74%94.19%111, 146, 25, 28, 56–57, 57, 57–58, 68, 78–79

Copy link
Contributor

@KevinFabre-ods KevinFabre-ods left a 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 😄

.github/workflows/visualizations.yml Outdated Show resolved Hide resolved
@@ -53,7 +51,7 @@
"trailingComma": "es5"
},
"dependencies": {
"@opendatasoft/visualizations": "0.21.1",
"@opendatasoft/visualizations": "^0.21.1-beta.0",
Copy link
Contributor

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

Suggested change
"@opendatasoft/visualizations": "^0.21.1-beta.0",
"@opendatasoft/visualizations": "0.21.1",

Copy link
Contributor

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

packages/visualizations/tsconfig.json Outdated Show resolved Hide resolved
packages/visualizations-react/.storybook/main.js Outdated Show resolved Hide resolved
Copy link

github-actions bot commented Apr 5, 2024

Coverage after merging chore/migrate-node-20 into main

94.64%

Coverage Report
FileBranchesFuncsLinesUncovered Lines
src
   index.ts100%100%100%
src/client
   error.ts100%100%100%
   index.ts74.03%100%95.31%102–103, 124, 13, 146, 148, 148–149, 15, 15, 151, 162, 169, 169, 17, 17, 171, 176, 179, 182, 184, 52, 82
   types.ts100%100%100%
src/odsql
   clauses.ts71.43%80%90.91%14, 32, 42
   index.ts83.72%95.74%94.19%111, 146, 25, 28, 56–57, 57, 57–58, 68, 78–79

1 similar comment
Copy link

github-actions bot commented Apr 5, 2024

Coverage after merging chore/migrate-node-20 into main

94.64%

Coverage Report
FileBranchesFuncsLinesUncovered Lines
src
   index.ts100%100%100%
src/client
   error.ts100%100%100%
   index.ts74.03%100%95.31%102–103, 124, 13, 146, 148, 148–149, 15, 15, 151, 162, 169, 169, 17, 17, 171, 176, 179, 182, 184, 52, 82
   types.ts100%100%100%
src/odsql
   clauses.ts71.43%80%90.91%14, 32, 42
   index.ts83.72%95.74%94.19%111, 146, 25, 28, 56–57, 57, 57–58, 68, 78–79

@etienneburdet etienneburdet force-pushed the chore/migrate-node-20 branch from 15cf2de to 33d08b9 Compare April 5, 2024 11:10
Copy link

github-actions bot commented Apr 5, 2024

Coverage after merging chore/migrate-node-20 into main

94.64%

Coverage Report
FileBranchesFuncsLinesUncovered Lines
src
   index.ts100%100%100%
src/client
   error.ts100%100%100%
   index.ts74.03%100%95.31%102–103, 124, 13, 146, 148, 148–149, 15, 15, 151, 162, 169, 169, 17, 17, 171, 176, 179, 182, 184, 52, 82
   types.ts100%100%100%
src/odsql
   clauses.ts71.43%80%90.91%14, 32, 42
   index.ts83.72%95.74%94.19%111, 146, 25, 28, 56–57, 57, 57–58, 68, 78–79

1 similar comment
Copy link

github-actions bot commented Apr 5, 2024

Coverage after merging chore/migrate-node-20 into main

94.64%

Coverage Report
FileBranchesFuncsLinesUncovered Lines
src
   index.ts100%100%100%
src/client
   error.ts100%100%100%
   index.ts74.03%100%95.31%102–103, 124, 13, 146, 148, 148–149, 15, 15, 151, 162, 169, 169, 17, 17, 171, 176, 179, 182, 184, 52, 82
   types.ts100%100%100%
src/odsql
   clauses.ts71.43%80%90.91%14, 32, 42
   index.ts83.72%95.74%94.19%111, 146, 25, 28, 56–57, 57, 57–58, 68, 78–79

@etienneburdet
Copy link
Contributor Author

I added an npm run build-ci script to install/build for all tasks, tell me what you think about it!

Copy link

Coverage after merging chore/migrate-node-20 into main

94.64%

Coverage Report
FileBranchesFuncsLinesUncovered Lines
src
   index.ts100%100%100%
src/client
   error.ts100%100%100%
   index.ts74.03%100%95.31%102–103, 124, 13, 146, 148, 148–149, 15, 15, 151, 162, 169, 169, 17, 17, 171, 176, 179, 182, 184, 52, 82
   types.ts100%100%100%
src/odsql
   clauses.ts71.43%80%90.91%14, 32, 42
   index.ts83.72%95.74%94.19%111, 146, 25, 28, 56–57, 57, 57–58, 68, 78–79

1 similar comment
Copy link

Coverage after merging chore/migrate-node-20 into main

94.64%

Coverage Report
FileBranchesFuncsLinesUncovered Lines
src
   index.ts100%100%100%
src/client
   error.ts100%100%100%
   index.ts74.03%100%95.31%102–103, 124, 13, 146, 148, 148–149, 15, 15, 151, 162, 169, 169, 17, 17, 171, 176, 179, 182, 184, 52, 82
   types.ts100%100%100%
src/odsql
   clauses.ts71.43%80%90.91%14, 32, 42
   index.ts83.72%95.74%94.19%111, 146, 25, 28, 56–57, 57, 57–58, 68, 78–79

@KevinFabre-ods KevinFabre-ods force-pushed the chore/migrate-node-20 branch from c34c893 to f76087d Compare April 12, 2024 09:03
Copy link

Coverage after merging chore/migrate-node-20 into main

94.64%

Coverage Report
FileBranchesFuncsLinesUncovered Lines
src
   index.ts100%100%100%
src/client
   error.ts100%100%100%
   index.ts74.03%100%95.31%102–103, 124, 13, 146, 148, 148–149, 15, 15, 151, 162, 169, 169, 17, 17, 171, 176, 179, 182, 184, 52, 82
   types.ts100%100%100%
src/odsql
   clauses.ts71.43%80%90.91%14, 32, 42
   index.ts83.72%95.74%94.19%111, 146, 25, 28, 56–57, 57, 57–58, 68, 78–79

1 similar comment
Copy link

Coverage after merging chore/migrate-node-20 into main

94.64%

Coverage Report
FileBranchesFuncsLinesUncovered Lines
src
   index.ts100%100%100%
src/client
   error.ts100%100%100%
   index.ts74.03%100%95.31%102–103, 124, 13, 146, 148, 148–149, 15, 15, 151, 162, 169, 169, 17, 17, 171, 176, 179, 182, 184, 52, 82
   types.ts100%100%100%
src/odsql
   clauses.ts71.43%80%90.91%14, 32, 42
   index.ts83.72%95.74%94.19%111, 146, 25, 28, 56–57, 57, 57–58, 68, 78–79

Copy link
Contributor

@KevinFabre-ods KevinFabre-ods left a 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

@KevinFabre-ods KevinFabre-ods merged commit 2e9803e into main Apr 12, 2024
11 checks passed
@KevinFabre-ods KevinFabre-ods deleted the chore/migrate-node-20 branch April 12, 2024 09:17
@KevinFabre-ods
Copy link
Contributor

@etienneburdet
Versions have been published, now we can update SDK packages on platform

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