Upgrade to node 20.17.0 lts #1000

munen · 2024-11-09T19:37:42Z

Closes: #992

W00t, this will be MR #1000! 🎊 🥳 💪

Builds upon:

These tasks should run:

CI needs to work

Upgrade Docker image for CI
CI should be green

Manual integration test

/sample still works
Logging into dropbox still works
Logging into webdav still works

Tests after deployment

Logging into gitlab still works
- I am assuming it'll work. I cannot login from localhost or tailscale hosts. Also on master. But the deployed organice.200ok.ch still works for me. So I'm assuming there's no change in this regard.

Follow Up todos after merging

The original PR https://github.com/200ok-ch/organice/pull/993/files still includes some upgrades (for example dropbox) that are not part of this PR. Some are, some are not. The reason for that is only that I haven't been able to add and test them, yet.

Notes:

For manual testing, this is deployed to https://staging.organice.200ok.ch/

My apologies, @schoettl, that I didn't finish the task in #993 where you've put in a lot of work 🙏 I actually did try quite hard. But I gave up after two hours. There were multiple errors that I didn't know how they were correlated. I then decided to take the task up from scratch, trying to do as small steps as possible and always checking all conditions for new errors. This still led to a lot of errors. And the PR did take me the better part of the day - even with the benefit of being able to look into your PR^^

I'd say this was a good combined effort. Thank you for giving organice the push to get some updates in! 🙇

We use sass only for the landing page. Versions >1.44.0 warn for an API change. But we only need to require one file. I didn't see how to use the new API without hitting the warning. We already just upgraded from node-sass to sass. So we can work with a slightly older new library, I'd say(;

One of your dependencies, babel-preset-react-app, is importing the "@babel/plugin-proposal-private-property-in-object" package without declaring it in its dependencies. This is currently working because "@babel/plugin-proposal-private-property-in-object" is already in your node_modules folder for unrelated reasons, but it may break at any time. babel-preset-react-app is part of the create-react-app project, which is not maintianed anymore. It is thus unlikely that this bug will ever be fixed. Add "@babel/plugin-proposal-private-property-in-object" to your devDependencies to work around this error. This will make this message go away.

munen · 2024-11-09T22:21:36Z

src/components/OrgFile/OrgFile.integration.test.js

@@ -26,15 +26,6 @@ import { STATIC_FILE_PREFIX } from '../../lib/org_utils';
 afterEach(cleanup);

 describe('Render all views', () => {
-  jest.mock('react-hotkeys', () => {


This code was throwing errors in yarn test. After fixing it, I wanted to know what it really does.

Result: I think this was not needed at all. The tests are still green(;

munen · 2024-11-09T22:23:00Z

package.json

    "@testing-library/jest-dom": "^5.14.1",
    "@testing-library/react": "12.0.0",
    "babel-eslint": "^10.1.0",
-    "eslint": "^6.6.0",
+    "eslint": "7.12.0",


CRA v4 uses eslint 7. I saw that you've went up to version 9. I tried to run 8 and 9, as well. And I think I did give it a good try. But I didn't manage it without errors. Probably it's because CRA v4 uses eslint 7. Don't know for sure, though.

munen · 2024-11-09T22:23:34Z

package.json

    "eslint-nibble": "^6.1.0",
    "eslint-plugin-jest": "^23.20.0",
    "eslint-plugin-react": "^7.23.2",
-    "eslint-plugin-react-redux": "^3.3.0",
+    "eslint-plugin-react-redux": "4.0.0",


Without that upgrade, there's a warning.

munen · 2024-11-09T22:24:21Z

package.json

    "redux": "^4.1.0",
    "redux-thunk": "^2.3.0",
-    "redux-undo": "1.0.1",
+    "redux-undo": "1.1.0",
+    "sass": "1.45.0",


Get rid of node-sass. sass is a JS only implementation of sass. There are newer versions, but 1.45.0 is from when CRA v4 was new.

munen · 2024-11-09T22:25:13Z

package.json

@@ -26,18 +28,20 @@
    "react-motion": "^0.5.2",
    "react-redux": "^7.2.1",
    "react-router-dom": "5.2.0",
-    "react-scripts": "3.4.3",
+    "react-scripts": "4.0.3",


I did try to go to v5. But I didn't manage it, yet. It's probably still a good idea. Or maybe switch to a different project like https://github.com/vitejs/vite.

munen · 2024-11-09T22:25:42Z

package.json

    "pegjs": "^0.10.0",
-    "react": "^16.8.3",
+    "react": "16.10.0",


Just a minimal update to get rid of a warning. Upgrading further is absolutely reasonable, of course!

schoettl

Hey @munen, great that you picked this up! I also gave up on my PR to be honest. Maybe we can cherry-pick some commits of it after this one is tested and merged.

schoettl · 2024-11-10T12:13:37Z

BTW, the node version you picked in this PR is exactly what I had in my nix flake, that's nice!

munen · 2024-11-11T07:54:32Z

Maybe we can cherry-pick some commits of it after this one is tested and merged.

Absolutely! There's still good stuff in there!

BTW, the node version you picked in this PR is exactly what I had in my nix flake, that's nice!

👍

Thank you for the blazing fast review and approval 🙏

munen added 8 commits November 9, 2024 17:33

feat: Upgrade node to 16.20.2

de73bd6

chore: Upgrade circleci node image

f6a0892

chore: Pin to the latest node v16 version available for CircleCI

fbc257a

feat: Upgrade CRA to v4.0.3

cbd7cf5

fix: Make eslint happy

10e3919

feat: Upgrade to Node 20.17.0

ce83d76

munen force-pushed the feat/upgrade-to-node-20.17.0-lts branch from 6e4bc78 to ce83d76 Compare November 9, 2024 19:39

munen added 2 commits November 9, 2024 21:15

fix: Upgrade eslint-plugin-react-redux to get rid of warning

ffc61c8

fix: yarn build works with manual sass compilation

04fd7f6

munen force-pushed the feat/upgrade-to-node-20.17.0-lts branch from b8b33a9 to 04fd7f6 Compare November 9, 2024 21:50

munen requested a review from schoettl November 9, 2024 21:53

chore: Upgrade redux-undo

f4ca1b2

munen commented Nov 9, 2024

View reviewed changes

schoettl approved these changes Nov 10, 2024

View reviewed changes

doc: Update changelog

8c0e157

munen merged commit 7c3d829 into master Nov 11, 2024
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade to node 20.17.0 lts #1000

Upgrade to node 20.17.0 lts #1000

munen commented Nov 9, 2024 •

edited

Loading

munen Nov 9, 2024

munen Nov 9, 2024

munen Nov 9, 2024

munen Nov 9, 2024

munen Nov 9, 2024

munen Nov 9, 2024

schoettl left a comment

schoettl commented Nov 10, 2024

munen commented Nov 11, 2024

Upgrade to node 20.17.0 lts #1000

Upgrade to node 20.17.0 lts #1000

Conversation

munen commented Nov 9, 2024 • edited Loading

CI needs to work

Manual integration test

Tests after deployment

Follow Up todos after merging

munen Nov 9, 2024

Choose a reason for hiding this comment

munen Nov 9, 2024

Choose a reason for hiding this comment

munen Nov 9, 2024

Choose a reason for hiding this comment

munen Nov 9, 2024

Choose a reason for hiding this comment

munen Nov 9, 2024

Choose a reason for hiding this comment

munen Nov 9, 2024

Choose a reason for hiding this comment

schoettl left a comment

Choose a reason for hiding this comment

schoettl commented Nov 10, 2024

munen commented Nov 11, 2024

munen commented Nov 9, 2024 •

edited

Loading