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

upgrade to node 20 #8322

Merged
merged 3 commits into from
Oct 25, 2023
Merged

upgrade to node 20 #8322

merged 3 commits into from
Oct 25, 2023

Conversation

RayBB
Copy link
Collaborator

@RayBB RayBB commented Sep 23, 2023

Closes #8257

This upgrades us to node 20!

Technical

We have to update bundlesize because it has a dependency that won't work with node 20. See this.

Testing

We have to rebuild the docker image and I'm having some trouble with that on my machine.
Maybe we can put it on testing?

Screenshot

Stakeholders

@cdrini

@RayBB RayBB changed the title upgrade node 20 upgrade to node 20 Sep 23, 2023
@RayBB RayBB marked this pull request as draft September 23, 2023 18:02
@RayBB
Copy link
Collaborator Author

RayBB commented Sep 26, 2023

bundlesize which relies on iltorb which is deprecated and doesn't work everywhere seems to cause the CI to fail. siddharthkp/bundlesize#320 (comment)

@codecov-commenter
Copy link

Codecov Report

Merging #8322 (55b1e3f) into master (1205b5f) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #8322   +/-   ##
=======================================
  Coverage   16.66%   16.66%           
=======================================
  Files          83       83           
  Lines        4422     4422           
  Branches      757      757           
=======================================
  Hits          737      737           
  Misses       3202     3202           
  Partials      483      483           

@RayBB RayBB marked this pull request as ready for review September 26, 2023 22:53
@mekarpeles mekarpeles added the Priority: 2 Important, as time permits. [managed] label Oct 2, 2023
@cdrini
Copy link
Collaborator

cdrini commented Oct 3, 2023

Awesome thank you @RayBB ! I'll need to find a way to build this image so you can test on it. Are you still having issues on mac? :/

Also just to confirm, is 20 the LTS version?

@RayBB
Copy link
Collaborator Author

RayBB commented Oct 3, 2023

@cdrini 20 will be the LTS version per this:

Node.js 20 will enter long-term support (LTS) in October, but until then, it will be the "Current" release

I'll try to build the image again and report the exact issue but until then I can use GitPod.

@RayBB
Copy link
Collaborator Author

RayBB commented Oct 3, 2023

@cdrini I was able to build it on GitPod (I didn't bother trying locally b/c I'm low on battery).
docker compose run --rm home node --version returns v20.8.0

Logging in works, editing author identifiers, the library explorer loads (but is empty as usual), and as far as I can tell everything seems fine. Is it possible to put this on testing for a few days and see if any issues pop up? Or are there any other parts we should explicitly test.

Because we use node/webpack just for building (and don't have a nodejs server) I feel issues should generally be caught at the build step if there were major ones.

@RayBB RayBB added this to Waiting Review/Merge from Staff in Ray's Project Oct 6, 2023
Copy link
Collaborator

@cdrini cdrini left a comment

Choose a reason for hiding this comment

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

Ok was able to successfully build/put on testing! LGTM ty @RayBB !

This might cause some headaches with package-lock.json files being out-of-sync/incorrect for folks. Will do another deploy tomorrow so this goes out and everyone can pull the latest version down and run a npm i --no-audit get everything.

@cdrini cdrini merged commit e4b9a48 into internetarchive:master Oct 25, 2023
4 checks passed
Ray's Project automation moved this from Waiting Review/Merge from Staff to Done Oct 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: 2 Important, as time permits. [managed]
Projects
Development

Successfully merging this pull request may close these issues.

Node 16 is end of life (EOL). Time to upgrade
4 participants