Skip to content

chore: update to node 22 #8101

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

Merged
merged 10 commits into from
May 9, 2025
Merged

chore: update to node 22 #8101

merged 10 commits into from
May 9, 2025

Conversation

snowystinger
Copy link
Member

Closes

You might notice that @react-aria/ssr still declares

  "engines": {
    "node": ">= 12"
  },

We can't update this without it no longer handling optional chaining, which webpack 4 cannot handle. So it remains at 12 for everyone and our code transpiles larger. 🤷🏻‍♂️

There is also now a deprecation warning for punycode, this will eventually go away most likely when storybook upgrade is complete and whenever we finally axe lerna.

✅ Pull Request Checklist:

  • Included link to corresponding React Spectrum GitHub Issue.
  • Added/updated unit tests and storybook for this change (for new code or code which already has tests).
  • Filled out test instructions.
  • Updated documentation (if it already exists for this component).
  • Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

🧢 Your Project:

devongovett
devongovett previously approved these changes Apr 17, 2025
@rspbot
Copy link

rspbot commented May 5, 2025

Base automatically changed from node-22 to main May 7, 2025 22:43
@dannify dannify dismissed devongovett’s stale review May 7, 2025 22:43

The base branch was changed.

@rspbot
Copy link

rspbot commented May 7, 2025

@rspbot
Copy link

rspbot commented May 7, 2025

@snowystinger snowystinger added this pull request to the merge queue May 9, 2025
Merged via the queue into main with commit ef27de0 May 9, 2025
30 checks passed
@snowystinger snowystinger deleted the node-22-real branch May 9, 2025 00:48
@nwidynski
Copy link
Contributor

nwidynski commented May 9, 2025

@snowystinger Btw, shouldn't the .nvmrc contain 22.x instead of 22.*? I don't think the star syntax is actually valid.

@snowystinger
Copy link
Member Author

snowystinger commented May 9, 2025

@nwidynski doesn't look like it

nvm use
Found '...react-spectrum/.nvmrc' with version <22.*>
Now using node v22.13.1 (npm v10.9.2)

vs

nvm use
Found '...react-spectrum/.nvmrc' with version <22.x>
N/A: version "22.x -> N/A" is not yet installed.

You need to run "nvm install 22.x" to install it before using it.

nvm install 22.x
Version '22.x' not found - try `nvm ls-remote` to browse available versions.

It doesn't actually seem to specify either syntax here https://github.com/nvm-sh/nvm?tab=readme-ov-file#nvmrc
but probably grabbed the star syntax from the lts/* example

Here's what their help says as well

Note: <version> refers to any version-like string nvm understands. This includes:
  - full or partial version numbers, starting with an optional "v" (0.10, v0.1.2, v1)
  - default (built-in) aliases: node, stable, unstable, iojs, system
  - custom aliases you define with `nvm alias foo`

Are you running into an issue using the current entry? or where are you coming up with the x syntax?

@nwidynski
Copy link
Contributor

nwidynski commented May 9, 2025

@snowystinger Meh, apparently both are valid, but even nvm isn't so sure https://github.com/nvm-sh/nvm?tab=readme-ov-file#set-default-node-version:~:text=the%20latest%20installed-,v18.x,-version%20of%20node themselves. I'm a bit surprised nvm install fails with .x syntax 😓

Im currently experiencing trouble with 20.* on supposedly .nvmrc compatible tooling, e.g. mise-en-place. The NPM examples also only mention .x syntax for minor versions 🤷 Regardless, I guess let's take the origin of .nvmrc as source of truth and just keep it that way.

@snowystinger
Copy link
Member Author

It looks like we don't actually need star syntax, we could just use the number 22. Would that work with mise-en-place?

@nwidynski
Copy link
Contributor

@snowystinger Yes 👍

@snowystinger
Copy link
Member Author

#8210

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.

5 participants