Skip to content

spike: update node 16 to node 18#2067

Merged
chrismclarke merged 49 commits intomasterfrom
spike/node-18
Jan 21, 2023
Merged

spike: update node 16 to node 18#2067
chrismclarke merged 49 commits intomasterfrom
spike/node-18

Conversation

@chrismclarke
Copy link
Member

@chrismclarke chrismclarke commented Jan 13, 2023

PR Checklist

PR Type

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Developer experience (improves developer workflows for contributing to the project)

Description

Add support for using Node 18. Note this is quite tricky due to the number of incompatibilities with webpack/node at different versions, and the way the monorepo is setup to handle specific package resolutions (see linked issue).

This PR aims to update everything to add compatibility for Node 18, and to migrate all of our cloud processes to use node 18 as a default. In addition it adds a prompt for local developers to try and match their environment to our recommendations

Dev Notes

We have numerous workspaces that all are pretty much all built using webpack, but in different ways (frontend via react-scripts, functions manual implementation, e2e tests via cypress, components storybook, docs via docusaurus). Some of these are setup to use webpack 4, others webpack 5. Trying to get everything upgraded to the newer version is a pain due to the large number of breaking changes, and the ways different packages either do or do not expose means to override configurations.

I think that I've now managed to update all the workspaces and resolve any identified issues to work with node 18 across the board. In the future we may want to consider restructuring the repo to keep better separation across workspaces (namely moving the platform frontend code away from the root folder), and possibly reconsidering some of our tooling options (particularly CRA which really failed in its support for webpack 5 and incredibly slow updates)

Review Notes

The automated tests should have caught any critical issues when migrating, however would be good to specifically check:

  • Checkout branch and try to run the platform (e.g. yarn start). It should prompt you to update version of node if you're not already running node 18. Be good to know how this experience is (are instructions clear? do things work?). Note the documentation page linked is updated in this PR so won't be entirely accurate

  • Once merged would also be good to check for any deployment issues and to check functions working as expected on dev site (e.g. adding vote to howto)

Followup-tasks

  • Release new emulators build with updated node version
  • Update firebase 8 -> 9 to reduce error logs (no longer strictly required, could be follow-up)
  • Consider downgrading circle-ci plan (not sure if we need anymore, should monitor resource usage)
  • Consider cra eject and/or implementing own unit test config/migrate to vitest (reduce cra lock-in and issues)

Tasks

Docs

  • Update documentation

CI

  • Update CI to node 18 (waiting release of 18.13.0 as per https://github.com/CircleCI-Public/cimg-node/pull/313)
  • Add node 16/18 matrix to ci tests
  • Reduce size of machine runners (previously needed medium+ for webpack4)
  • Remove ci=false and node_options from build-context (included locally instead)
  • Fix unit test memory leak issues (partial fix)

Frontend

  • Update frontend cra 4 -> 5
  • Add Craco to allow CRA webpack config overrides
  • Fix SVG issue
  • Ensure changed svgs look correct (and consider optimising all svg files)
  • Fix Uppy issue (https://github.com/transloadit/uppy/issues/3376)
  • Fix pino-logflare integration (will need to either eject or add craco to include stream polyfill)
  • Fix frontend build/run
  • Fix any other missing polyfills (e.g. webpack4 auto polyfills

Functions

  • Update functions webpack 4 -> 5
  • Fix functions build

Storybook

  • Fix storybook build (has webpack 5 dep but core not configured to use correctly)

Cypress E2E

  • Upgrade cypress to use webpack 5
  • Fix tests (cypress expects polyfill for crypto, also uppy issue below)

Emulators

  • Fix emulators build (pubsub issue linked to https://github.com/micrometer-metrics/micrometer/issues/2776)
  • Release new emulator build with updated firebase dep
  • Fix sourcemap warnings (https://github.com/facebook/create-react-app/discussions/11767)

Misc

  • Remove all top-level package.json resolutions (applied to child workspaces unexpectedly)
  • Re-test running platform with node 16 locally
  • Use healthcheck to enforce devs use node 18

Git Issues

Closes #2057

Screenshots/Videos

Message shown to users using older versions of node
image

Additional CI tests for legacy and next node versions (passing)
https://app.circleci.com/pipelines/github/ONEARMY/community-platform/3644/workflows/76bb0c89-3acb-4d31-a7ac-654639a643c6
image


What happens next?

Thanks for the contribution! We try to make sure all PRs are reviewed ahead of a monthly dev call (first Monday of the month, open to all!).

If the PR is working as intended it'll be merged and included in the next platform release, if not changes will be requested and re-reviewed once updated.

If you need more immediate feedback you can try reaching out on Discord in the Community Platform development channel.

@chrismclarke chrismclarke changed the title chore: update node 16 to node 18 spike: update node 16 to node 18 Jan 15, 2023
@cypress
Copy link

cypress bot commented Jan 15, 2023



Test summary

52 0 0 0Flakiness 0


Run details

Project onearmy-community-platform
Status Passed
Commit 6b6feee
Started Jan 21, 2023 2:40 AM
Ended Jan 21, 2023 2:43 AM
Duration 02:47 💡
OS Linux Ubuntu -
Browser Chrome 109

View run in Cypress Dashboard ➡️


This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

@chrismclarke chrismclarke marked this pull request as ready for review January 17, 2023 03:31
@chrismclarke chrismclarke requested a review from a team as a code owner January 17, 2023 03:31
Copy link
Contributor

@davehakkens davehakkens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for all the work on this Chris.
Updated Node to 18, did a yarn start and it all works.
Very smooth, seems good here!

Copy link
Contributor

@evakill evakill left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wasn't getting the print statements on yarn start - found a console.log that needed to be pulled out of the if/else in the envCheck

otherwise, LGTM and runs smoothly when using node 18 🎉

chrismclarke and others added 2 commits January 20, 2023 18:30
Co-authored-by: Eva Killenberg <eva@replicahq.com>
@chrismclarke
Copy link
Member Author

Think this one is (finally) good to go.

@davehakkens I'll give a double-check to the deployed dev sites post-merge, but if you could give me a heads up when you are planning to make the next release and I wouldn't mind also quickly checking the production sites at the same time (around most this weekend or Monday on my usual timezone).

@chrismclarke chrismclarke merged commit 7073785 into master Jan 21, 2023
@chrismclarke chrismclarke deleted the spike/node-18 branch January 21, 2023 02:51
@davehakkens
Copy link
Contributor

they al seems to work well @chrismclarke.

Feel free the push the release at your convience, so you can double check.
I dont have any specific time for it that it needs to go out.
Now/this weekend seems like a good moment.

@onearmy-bot
Copy link
Collaborator

🎉 This PR is included in version 1.35.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@thisislawatts
Copy link
Contributor

@chrismclarke are the deployed instances all upgraded to Node 18 now as well? If so can we remove the node 16 build job from the CircleCI workflow?

@chrismclarke
Copy link
Member Author

They should all be, I had kept in when planning to keep the node 18 usage optional but now its enforce fine to get rid.
I'm guessing we might also want to pin to a specific version of node 18 instead of just current (?)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

[feature request] Upgrade project to Node 18

5 participants