Skip to content

Update node to v22#5378

Merged
julienw merged 2 commits into
firefox-devtools:mainfrom
julienw:update-node-v22
Feb 21, 2025
Merged

Update node to v22#5378
julienw merged 2 commits into
firefox-devtools:mainfrom
julienw:update-node-v22

Conversation

@julienw

@julienw julienw commented Feb 20, 2025

Copy link
Copy Markdown
Contributor

This updates node to version 22. This only needed very few changes in the tests.

@julienw julienw force-pushed the update-node-v22 branch 2 times, most recently from a5005b2 to 584616d Compare February 20, 2025 11:36
@codecov

codecov Bot commented Feb 20, 2025

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.96%. Comparing base (770a1d0) to head (584616d).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5378   +/-   ##
=======================================
  Coverage   85.96%   85.96%           
=======================================
  Files         312      312           
  Lines       30330    30330           
  Branches     8296     8296           
=======================================
  Hits        26074    26074           
  Misses       3659     3659           
  Partials      597      597           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@julienw julienw requested a review from canova February 20, 2025 11:47
Comment thread .nvmrc Outdated
@@ -1 +1 @@
18.19
22

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should we also put 22.14 here specifically?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This question sparked a lot of conflicting thoughts :-D

  1. Initially I was thinking that nvm would install the latest version which will always be >= 22.14, but if the user already has an earlier v22 it would indeed fail later.
  2. Then I tried locally: nvm use doesn't install it automatically, it only tries to use it if available. So in that case leaving it at 22 would probably be better. For example that would let a user pick a later version too.
  3. However I believe we don't use nvm use ourselves without a version number, we mostly use a version number explicitely. So this .nvmrc is more a hint to a user. But, more importantly, it is used by netlify to pick a node version (netlify will install it if it's not present).

Looking at the history of .nvmrc, I'm gonna use the more explicit version name like you suggested. This way we control the node version we know it's working for generating the project.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I wonder if I should use 22.14 also in package.json, given this is the version checked by bin/pre-install.js... what do you think?
Or maybe we can remove it from pre-install.js at all, if this is checked by yarn directly now?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We can do that as a follow-up, I'm gonna merge this for now :-)

@julienw julienw enabled auto-merge February 21, 2025 10:45
@julienw julienw merged commit 263ee2b into firefox-devtools:main Feb 21, 2025
@julienw julienw mentioned this pull request Mar 3, 2025
julienw added a commit that referenced this pull request Mar 3, 2025
[Julien Wajsberg] Update node to v22 (#5378)
[Markus Stange] Speed up createCallNodeTable by 2.3x (#5248)
[Markus Stange] Remove frameTable.implementation from the processed format. (#5370)
[Florian Quèze] Show size units in the timeline for profiles where profile.meta.sampleUnits.time is "bytes". (#5364)
[Sean Kim] Report nsIRequest::status (nsresult) in the marker (#5375)
[Markus Stange] Change the marker schema to accept a description and get rid of the notion of static fields (#5385)

Also thanks to our localizers:
en-CA: Paul
tr: Grk
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants