ONEARMY / community-platform

A platform to build useful communities that aim to tackle global problems
https://platform.onearmy.earth
MIT License
1.14k stars 386 forks source link

[feature request] Upgrade project to Node 18 #2057

Closed thisislawatts closed 1 year ago

thisislawatts commented 1 year ago

Is your feature request related to a problem? Please describe.

The current version of Node LTS (Long Term Support) is version 18. The project's current version 16 transitioned to maintenance only mode in October 2022.

A few places to check compatability:

[ ] Circle CI runners [ ] GitHub workers [ ] Google Cloud Functions

chrismclarke commented 1 year ago

I might try take a quick look at this (came across some node 18 issues previously, mentioned in #2033)

chrismclarke commented 1 year ago

So taking a quick look I can see a couple issues where build/start scripts fail due to incompatibilities between node 18 and react-scripts 4 (/webpack, 4 also used for functions build)

These can be fixed by adding flags (e.g. react-scripts --openssl-legacy-provider --no-experimental-fetch build), BUT these flags then fail if trying to run in node 16. So I think this leaves a couple options:

  1. Force all devs to upgrade to node 18. We could make this more obvious in the short term by including a check before running start command that outputs a friendly upgrade message if using other versions. We have some infrastructure for this already from #2033, just need to call it before start script

  2. Maintain alternate scripts for core commands (probably just start). Use something like if-node-version to only add flags when running node 18. We would keep CI commands node-18 only

  3. Push forward with core updates for either webpack/react-scripts 5 or vite (some work done in #2031), in the hopes that they maintain better backwards compatibility

Any thoughts @thisislawatts ?

Related PR for ref: #2067

chrismclarke commented 1 year ago

Update, a few related tasks/knock-ons likely required to get things working more smoothly for given routes:

Update platform to webpack 5 (react-scripts 5)

Update functions to webpack 5

Misc

chrismclarke commented 1 year ago

Another day, another set of issues. Most of them come down to webpack inconsistencies for version 4/5 with node 16/18. Running yarn why webpack you can see other version used to bundle various components:

docs (docusaurus): 5.75.0 components (storybook): 4.46.0 functions (manual): 4.46.0 (spiked out to 5.75.0) src (react-scripts): 4.44.2

Ordinarly we can work with different versions of webpack in different packages without much issue, however for reasons I can't quite figure out yarn defaults to hoisting webpack 5 to use on all packages unless we set a resolutions field in the main package.json to apply webpack 4, however that then imposes webpack 4 on all other workspaces (likely because the frontend isn't really a proper workspace but at the root level). I've still got a bit of figuring out to do, but my current plan is:

chrismclarke commented 1 year ago

Stream of consciousness entry no. 5

The issue of resolutions makes broad sense now, only the top-level package.json resolutions will be respected and ordinarily would be imposed on all child packages. This essentially means that our main src and functions repos need to run the same webpack version (docusaurus/storybook get around this by including standalone webpack modules within their own core modules, react-scripts tries to do the same but then still defers to the top-level package due to... reasons...)

Updating functions to webpack 5 has been smooth, react-scripts less so however I'm not down to just a single sticking point (pino-logflare uses streams which require a polyfill that webpack 4 included but webpack 5 does not, and react-scripts does not expose the webpack config to manually now). So the interim solution is just to disable the pino-logflare integration (not sure if we are even using?), and add a follow-up issue if required to bring back in either by ejecting the react-scripts webpack config (would require a small refactor of our existing scripts folder as that is the default location react ejects to, so needs to be empty), or using a tool like craco to just provide partial webpack override.

There's still the rest of the moving pieces to test out, but I think moving forward this approach seems preferable to either a) giving up and telling everyone to stick to node 16, or b) tearing the entire system apart to replace with something else.

chrismclarke commented 1 year ago

Now on to storybook, similar situation (in that it's a confusing mess), we have webpack5 installed in the workspace however it seems like most likely webpack4 in use. Various comments/issues mentioned in:

https://gist.github.com/shilman/8856ea1786dcd247139b47b270912324#upgrade and https://github.com/storybookjs/storybook/issues/14044

Again preference here is to bump to webpack5 for consistency and interoperability with node 18

chrismclarke commented 1 year ago

Storybook fixed, onto cypress testing and more issues because of the way it references variables that were previously polyfilled (e.g. crypto, process). Possibly can be fixed with a few manual globalThis checks, otherwise eject/craco + polyfill.

Still a handful of issues to resolve so going to move to tracking as TODOs in the related PR

chrismclarke commented 1 year ago

Think I've finally finished off the PR, a few takeaways from the overall experience:

onearmy-bot commented 1 year ago

:tada: This issue has been resolved in version 1.35.0 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket: