Skip to content

[CI] bump Node references from 12/14 to 14/16#32980

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.

Sign up for GitHub

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

Closed
kelset wants to merge 1 commit into from
Closed

[CI] bump Node references from 12/14 to 14/16 #32980

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions .circleci/config.yml
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -28,11 +28,11 @@ executors:
<<: *defaults
docker:
# Note: Version set separately for Windows builds, see below.
- image: circleci/node:14
- image: circleci/node:16
nodeprevlts:
<<: *defaults
docker:
- image: circleci/node:12
- image: circleci/node:14
reactnativeandroid:
<<: *defaults
docker:
Expand DownExpand Up@@ -376,7 +376,7 @@ jobs:
- run:
name: Configure Environment Variables
command: |
echo 'export PATH=/usr/local/opt/node@14/bin:$PATH' >> $BASH_ENV
echo 'export PATH=/usr/local/opt/node@16/bin:$PATH' >> $BASH_ENV
source $BASH_ENV

- with_brew_cache_span:
Expand All@@ -393,7 +393,7 @@ jobs:
name: Configure Node
# Sourcing find-node.sh will ensure nvm is set up.
# It also helps future invocation of find-node.sh prevent permission issue with nvm.sh.
command: source scripts/find-node.sh && nvm install 14 && nvm alias default 14
command: source scripts/find-node.sh && nvm install 16 && nvm alias default 16

- run:
name: Configure Watchman
Expand DownExpand Up@@ -644,8 +644,8 @@ jobs:
name: Install Node
# Note: Version set separately for non-Windows builds, see above.
command: |
nvm install 14.17.0
nvm use 14.17.0
nvm install 16
nvm use 16

# Setup Dependencies
- run:
Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -7,7 +7,7 @@
"license": "MIT",
"repository": "github:facebook/react-native",
"engines": {
"node": ">=12"
"node": ">=14"

@NickGerlemanNickGerlemanJan 27, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This will create a warning or error for users of RN with Node 12. Is it okay to make the breaking change yet?

@SaadnajmiSaadnajmiJan 27, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Also, why 14 and not 16?

@kelsetkelsetJan 28, 2022•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

@NickGerleman I think it's ok to make it a breaking change, folks should be on current LTS (so 16).

@Saadnajmi it's 14 because on CircleCI we also have a job that tests against the previous LTS (14) hence why the requirement. You can see me bumping that nodeprevlts in this PR

@cortinicocortinicoJan 28, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@NickGerleman I think it's ok to make it a breaking change, folks should be on current LTS (so 16).

Agree here. Plus the current Node version (non lts) is 17.
I think it's totally ok to drop Node 12 (maybe we should add a note in the Changelog for this).

@kelsetkelsetJan 28, 2022

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

yeah let me change the changelog entry

@ShikaSDShikaSDJan 28, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Seems like the changelog entry didn't sync properly, we might need to update this separately when building the changelog

@kelsetkelsetJan 28, 2022

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

@ShikaSD interesting - is this because I changed the text of the PR body after the import process was started?

@ShikaSDShikaSDJan 28, 2022•
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we landed the previous version internally, so we didn't re-import the description

@kelsetkelsetJan 28, 2022

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

gotcha - makes sense.

},
"jest-junit": {
"outputDirectory": "reports/junit",
Expand Down
2 changes: 1 addition & 1 deletion scripts/validate-ios-test-env.sh
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -23,7 +23,7 @@ fi
# Check that the correct version of node is installed
NODE_VERSION="$(command node --version | sed 's/[-/a-zA-Z]//g' |sed 's/.\{2\}$//')"

if (( $(echo "${NODE_VERSION} < 12.0" | bc -l) )); then
if (( $(echo "${NODE_VERSION} < 14.0" | bc -l) )); then
echo "Node ${NODE_VERSION} detected. This version of Node is not supported."
echo "See https://reactnative.dev/docs/getting-started for instructions."
exit 1
Expand Down