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

[DO NOT MERGE] Test node 22 #2133

Open
wants to merge 13 commits into
base: develop
Choose a base branch
from
Open

[DO NOT MERGE] Test node 22 #2133

wants to merge 13 commits into from

Conversation

alexvuong
Copy link
Collaborator

Description

Types of Changes

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Documentation update
  • Breaking change (could cause existing functionality to not work as expected)
  • Other changes (non-breaking changes that does not fit any of the above)

Breaking changes include:

  • Removing a public function or component or prop
  • Adding a required argument to a function
  • Changing the data type of a function parameter or return value
  • Adding a new peer dependency to package.json

Changes

  • (change1)

How to Test-Drive This PR

  • (step1)

Checklists

General

  • Changes are covered by test cases
  • CHANGELOG.md updated with a short description of changes (not required for documentation updates)

Accessibility Compliance

You must check off all items in one of the follow two lists:

  • There are no changes to UI

or...

Localization

  • Changes include a UI text update in the Retail React App (which requires translation)

@alexvuong alexvuong added the do not merge No matter what, do not merge this pr label Nov 14, 2024
@alexvuong alexvuong self-assigned this Nov 14, 2024
@alexvuong alexvuong requested a review from a team as a code owner November 14, 2024 22:11
@alexvuong alexvuong added the skip changelog Skip the "Changelog Check" GitHub Actions step even if the Changelog.md files are not updated label Nov 14, 2024
- node: 16
npm: 10
node: [20, 22]
npm: [10, 10]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Node 18 is still in LTS for 5 more months. What is the timeline for this upgrade?

Copy link
Collaborator Author

@alexvuong alexvuong Nov 21, 2024

Choose a reason for hiding this comment

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

Good call. I believe we have to wait til MRT officially supports Node 22. I think at least one more RP to start this upgrade. I think it is safe to remove 16 for this upgrade since it is already out of support timeline. I'll leave 18 and will remove if it is out of support timeline by time we officially do the upgrade

- node: 16
npm: 10
node: [20, 22]
npm: [10, 10]
runs-on: ubuntu-latest
env:
# The "default" npm is the one that ships with a given version of node.
# For more: https://nodejs.org/en/download/releases/
# (We also use this env var for making sure a step runs once for the current node version)
# Note: For node 18, the default was npm 9 until v18.19.0, when it became npm 10
Copy link
Collaborator

Choose a reason for hiding this comment

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

If we end up moving forward. Don't forget to remove comments about unsupported versions.

@@ -32,7 +32,7 @@
"syncpack": "^10.1.0"
},
"engines": {
"node": "^16.11.0 || ^18.0.0 || ^20.0.0",
"node": "^16.11.0 || ^18.0.0 || ^20.0.0 || ^22.0.0",
Copy link
Collaborator

Choose a reason for hiding this comment

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

As an aside, I wonder if we should simply adopt nodes support model or a modified version of it, e.g. we only support active node LTS. Then at most we'll probably have 3 versions here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Although some Node version may be in maintenance but they are still within Life-of-support timeline. IMO, when we do upgrade, we can simply drop the node versions that has no longer support officially. That would also leave us with around 2-3 version here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do not merge No matter what, do not merge this pr skip changelog Skip the "Changelog Check" GitHub Actions step even if the Changelog.md files are not updated
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants