Skip to content
This repository has been archived by the owner on Apr 19, 2024. It is now read-only.

Allow building with modern NodeJS #467

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

peat-psuwit
Copy link

@peat-psuwit peat-psuwit commented Sep 26, 2023

The old devDependencies of this package causes 2 problems:

  • It makes building this package in the distributions difficult. Many distros no longer package NodeJS 12 that this project targeted.
  • The netdata/devenv package has its NodeJS upgraded to version 20 recently [1]. This breaks development of this package.

This PR aims to modernize the build-time dependencies so that it can be built with modern version of NodeJS (version 18 and newer). This will break support of NodeJS 12 this project is currently using. However, since NodeJS 12 is EOL'ed for over a year already, I think it's time to move on.

Updating the runtime dependencies of this project is not the aim of this PR.

Note that due to behavior changes in NPM v7 and above [2], we need to use --legacy-peer-deps flag every time we install packages with NPM. This boils down to the conflict between @netdata/react-filter-box's dependencies which contain react@^16.6.3, and @netdata/netdata-ui's peerDependencies which contains react@^18.2.0 (even in version 2.0.0 specified in package-lock.json). Because of this conflict, we can't just raise this package's peerDependencies of react, and will have to fix the dependencies of @netdata/react-filter-box first.

[1] netdata/community#85
[2] https://stackoverflow.com/a/66620869

This is done by running `npm install --package-lock-only --legacy-peer-
deps` with NPM version 10.1.0. This is done to make further changes to
the lockfile be visible, and shouldn't change the meaning of the lock
file.
These modules (canvas and node-sass) are the minimum set of
dependencies required to be upgraded for the dependencies to be
installable.
@peat-psuwit
Copy link
Author

At the moment I can get dependencies to install on NodeJS 20. I've upgraded react-scripts, Typescript to the latest versions, fixed various typing errors, and fixed Webpack errors. I'm still stuck with ESLint dependencies (I suspect I have to upgrade all of them & fix various errors that will appear), but I'm out of time now so I'll leave it here in case someone is interested in continuing the work. If I happen to find some time again, I might return to continue this work.

@peat-psuwit peat-psuwit changed the title Allow building with modern NodeJS WIP: Allow building with modern NodeJS Sep 26, 2023
@peat-psuwit peat-psuwit force-pushed the build-with-modern-node branch 2 times, most recently from ea479c7 to cdbc95c Compare September 28, 2023 04:53
It mutates node_modules directory in an unpredictable way, and it's
no longer needed since NPM 7 and above now automatically installs
peer dependencies.
This is done to bring in newer sass-loader, compatible with node-sass
9.0. And since CRACO is intimately linked to react-scripts, it has to be
upgraded in lock step.
This is required by certain types pulled in as transitive depenencies of
newer react-scripts.
Required by the upgrade of Typescript.
- Specify the type of `useState()` data as `any`. Unfortunately, all of
  usages can't be typed properly, as their actual types are either not
  imported correctly, or not imported at all.
- Ignore the type of Redux DevTools middleware, since I don't know how
  it's typed exactly.
- Remove type coercion in units-conversions.ts.
- Workaround TS9005 error due to d3pie being vendored by providing a
  fake JSDoc comment.
With all dependency upgrade I've done, the support for NodeJS 12 is now
broken. So it's time to move on to a newer, still supported version.

NodeJS 20 is choosen because a.) while it's not an LTS yet, it will
become an LTS soon, and b.) it matches the version in netdev/devenv
Docker image.
@peat-psuwit peat-psuwit changed the title WIP: Allow building with modern NodeJS Allow building with modern NodeJS Sep 28, 2023
@peat-psuwit peat-psuwit marked this pull request as ready for review September 28, 2023 05:42
@peat-psuwit
Copy link
Author

peat-psuwit commented Sep 28, 2023

I now manage to build this package under NodeJS 20 and NodeJS 18. I quickly test of the dashboard and it seems to work. So I believe this is ready for review.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant