Skip to content

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

Closed
kelset wants to merge 1 commit into
mainfrom
chore/bump-node-to-16
Closed

[CI] bump Node references from 12/14 to 14/16#32980
kelset wants to merge 1 commit into
mainfrom
chore/bump-node-to-16

Conversation

@kelset

@kelsetkelset commented Jan 27, 2022

Copy link
Copy Markdown
Contributor

Summary

Node 16 has been the LTS for quite some time now (Oct 2021), so this PR just wants to bring the RN OSS CI up to speed.

(I realized that this was needed while doing the same for macos microsoft#997)

Changelog

[General] [Changed] - Breaking: moving minimum version of Node expected to 14, and move CI to current LTS(16).

Test Plan

CI itself is the test 🤣 locally no significant changes were experienced.

@kelset
kelset requested a review from hramos as a code ownerJanuary 27, 2022 17:14
@facebook-github-botfacebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Contributor A React Native contributor. p: Microsoft Partner: Microsoft Partner Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Jan 27, 2022
@facebook-github-bot

Copy link
Copy Markdown
Contributor

@ShikaSD has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

Comment threadpackage.json
"repository": "github:facebook/react-native",
"engines": {
"node": ">=12"
"node": ">=14"

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?

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

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

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).

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

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

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

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

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.

@cortinicocortinico left a comment

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.

Change looks go to me 👍 You can disregard the internal failure as it's an unrelated warning (that gets exported as a error on Github Status).

@react-native-bot

Copy link
Copy Markdown
Collaborator

This pull request was successfully merged by @kelset in f1488db.

When will my fix make it into a release? | Upcoming Releases

@react-native-botreact-native-bot added the Merged This PR has been merged. label Jan 28, 2022
@kelset
kelset deleted the chore/bump-node-to-16 branch January 28, 2022 13:26
amgleitman pushed a commit to amgleitman/react-native-macos that referenced this pull request Apr 13, 2022
Summary:
Node 16 has been the LTS for quite some time now ([Oct 2021](https://nodejs.org/en/blog/release/v16.13.0/)), so this PR just wants to bring the RN OSS CI up to speed.
(I realized that this was needed while doing the same for macos microsoft#997)
<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://github.com/facebook/react-native/wiki/Changelog
-->
[General] [Changed] - CI moved to Node 16.
Pull Request resolved: react#32980
Test Plan: CI itself is the test � locally no significant changes were experienced.
Reviewed By: cortinico
Differential Revision: D33821288
Pulled By: ShikaSD
fbshipit-source-id: 4480ae1cb909e2b8d0b6abba4db76bbe71f0193e
amgleitman added a commit to microsoft/react-native-macos that referenced this pull request Apr 13, 2022
amgleitman pushed a commit to amgleitman/react-native-macos that referenced this pull request Apr 13, 2022
Summary:
Node 16 has been the LTS for quite some time now ([Oct 2021](https://nodejs.org/en/blog/release/v16.13.0/)), so this PR just wants to bring the RN OSS CI up to speed.
(I realized that this was needed while doing the same for macos microsoft#997)
<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://github.com/facebook/react-native/wiki/Changelog
-->
[General] [Changed] - CI moved to Node 16.
Pull Request resolved: react#32980
Test Plan: CI itself is the test � locally no significant changes were experienced.
Reviewed By: cortinico
Differential Revision: D33821288
Pulled By: ShikaSD
fbshipit-source-id: 4480ae1cb909e2b8d0b6abba4db76bbe71f0193e
facebook-github-bot pushed a commit that referenced this pull request Nov 25, 2022
Summary:
Node 18: https://nodejs.org/de/blog/announcements/v18-release-announce/
Node 16 EOL: 2023-09-11
https://nodejs.org/en/blog/announcements/nodejs16-eol/
Node 18 EOL: 2025-04-30
Follow-up
- #34171
- #32980
Ref
- react-native-community/docker-android#187
cc ramonmedel cortinico gengjiawen
## Changelog
<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->
[JavaScript] [Changed] - Bump node version from 16 to 18
Pull Request resolved: #35443
Test Plan: Everything builds and runs as expected
Reviewed By: cipolleschi
Differential Revision: D41531161
Pulled By: cortinico
fbshipit-source-id: 305888f55ed179f75bef34548aebf22fc2951308
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA SignedThis label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.ContributorA React Native contributor.MergedThis PR has been merged.p: MicrosoftPartner: MicrosoftPartnerShared with MetaApplied via automation to indicate that an Issue or Pull Request has been shared with the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@kelset@facebook-github-bot@react-native-bot@NickGerleman@cortinico@ShikaSD@Saadnajmi