-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
ci: bump node v22 #30251
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
ci: bump node v22 #30251
Conversation
|
|
WalkthroughThe pull request introduces updates to multiple GitHub Actions workflow files, primarily changing the Node.js version from Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant GitHub Actions
participant Node.js
User->>GitHub Actions: Trigger Workflow
GitHub Actions->>Node.js: Setup Node.js v22
Node.js-->>GitHub Actions: Node.js v22 Ready
GitHub Actions->>GitHub Actions: Execute Workflow Steps
GitHub Actions-->>User: Workflow Complete
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (2)
.github/workflows/changelog.yml (1)
Line range hint
24-24: Consider centralizing Node.js version management.To maintain consistency and simplify future updates, consider:
- Using a shared environment variable or GitHub Actions variable for the Node.js version
- Creating a reusable workflow for Node.js setup steps
Example implementation:
# .github/workflows/setup-node.yml name: Reusable Node.js Setup on: workflow_call: inputs: node-version: default: '22' type: string jobs: setup: steps: - uses: actions/setup-node@v4 with: node-version: ${{ inputs.node-version }}Also applies to: 32-32, 31-31
.github/workflows/release.yml (1)
Line range hint
27-29: Critical: Node.js version not updated to v22The Node.js version is still set to v20, which is inconsistent with the PR objective to upgrade to v22. This could lead to inconsistencies across workflows.
Apply this diff to maintain consistency:
with: - node-version: 20 + node-version: 22 registry-url: "https://registry.npmjs.org/"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.github/workflows/autofix-docs.yml(2 hunks).github/workflows/autofix.yml(2 hunks).github/workflows/benchmark.yml(1 hunks).github/workflows/changelog.yml(1 hunks).github/workflows/ci.yml(1 hunks).github/workflows/docs.yml(1 hunks).github/workflows/lint-monorepo.yml(1 hunks).github/workflows/release.yml(1 hunks)
🔇 Additional comments (5)
.github/workflows/autofix-docs.yml (1)
24-24: LGTM! Node.js version update is appropriate.
The upgrade to Node.js 22 is suitable as it's the latest stable version, and all workflow tasks are compatible with this version.
.github/workflows/changelog.yml (1)
31-31: LGTM! Node.js version update is consistent.
The upgrade to Node.js 22 aligns with the project-wide update, and the changelog generation tasks are compatible with this version.
.github/workflows/docs.yml (1)
28-28: LGTM: Node.js version update
The update to Node.js v22 aligns with the PR objectives and uses the latest stable version.
.github/workflows/autofix.yml (1)
20-20: LGTM: Node.js version update
The update to Node.js v22 aligns with the PR objectives and maintains workflow consistency.
.github/workflows/benchmark.yml (1)
36-36: LGTM! Verify CodSpeedHQ compatibility with Node.js 22
The update to Node.js 22 looks good. Since this workflow uses CodSpeedHQ for benchmarking, let's verify its compatibility.
| - uses: actions/setup-node@39370e3970a6d050c480ffad4ff0ed4d3fdee5af # v4.1.0 | ||
| with: | ||
| node-version: 20 | ||
| node-version: 22 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Update Node.js version requirements in schema package
The monorepo has inconsistent Node.js version requirements. While most packages support Node.js 22 (>=22.0.0), the following packages have outdated requirements:
packages/schema/package.json: "^14.18.0 || >=16.10.0"package.json(root): "^16.10.0 || >=18.0.0"
Please update these packages to align with the Node.js 22 requirement in the CI workflow.
🔗 Analysis chain
Verify monorepo package compatibility with Node.js 22.
While the Node.js version update is appropriate, please ensure all packages in the monorepo support Node.js 22 in their engines field.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check Node.js version requirements in all package.json files
# Find all package.json files and check their engines field
fd package.json --type f --exclude node_modules --exec sh -c 'echo "=== $1 ==="; cat "$1" | jq -r ".engines.node // \"No engine specified\""' sh {}
Length of output: 1430
.github/workflows/release.yml
Outdated
| id-token: write | ||
| runs-on: ubuntu-latest | ||
| timeout-minutes: 20 | ||
| timeout-minutes: 22 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Timeout increase appears unnecessary without clear justification
The timeout was indeed increased from 20 to 22 minutes, but there's no evidence of workflow failures or timeouts that would justify this change. Additionally, since the Node.js version remains at v20 in the workflow, this timeout increase doesn't appear to be correlated with any Node.js version changes.
- Consider reverting the timeout back to 20 minutes unless there's a specific reason for the increase that hasn't been documented
- If there are specific timeout issues being addressed, please document them in the commit message or PR description
🔗 Analysis chain
Consider if timeout increase is necessary
The timeout has been increased from 20 to 22 minutes. Please clarify if this increase is related to the Node.js version update or if it's addressing a specific issue.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check recent workflow runs to see if they're timing out
gh run list --workflow=release.yml --limit 10 --json conclusion,durationMs | \
jq '.[] | select(.conclusion=="failure") | .durationMs'
Length of output: 370
Script:
#!/bin/bash
# Check recent workflow runs with available fields
gh run list --workflow=release.yml --limit 10 --json conclusion,startedAt,updatedAt | \
jq '.[] | select(.conclusion=="failure") | {conclusion: .conclusion, duration: (((.updatedAt | fromdateiso8601) - (.startedAt | fromdateiso8601))/60)}'
# Get the previous timeout value from git history
git log -p .github/workflows/release.yml | grep -A 1 "timeout-minutes:" | head -n 2
Length of output: 239
Script:
#!/bin/bash
# Let's check the workflow file content and its recent changes
git show HEAD:.github/workflows/release.yml
git show HEAD^:.github/workflows/release.yml | grep "timeout-minutes:"
Length of output: 1298
.github/workflows/ci.yml
Outdated
| - uses: actions/setup-node@39370e3970a6d050c480ffad4ff0ed4d3fdee5af # v4.1.0 | ||
| with: | ||
| node-version: 20 | ||
| node-version: 22 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Critical: Node.js version inconsistency confirmed in CI workflow
The verification confirms that the CI workflow (ci.yml) has inconsistent Node.js versions:
- One job using Node.js 22
- Five jobs using Node.js 20
This inconsistency needs to be addressed, especially since most other workflow files (docs.yml, lint-monorepo.yml, changelog.yml, autofix.yml, autofix-docs.yml, benchmark.yml) have already been updated to use Node.js 22.
Update all jobs in ci.yml to use Node.js 22 for consistency:
# Update in all job sections:
- node-version: 20
+ node-version: 22🔗 Analysis chain
Critical: Inconsistent Node.js versions across jobs
The build job has been updated to Node.js 22, but other jobs (typecheck, lint, test-unit, build-release, release-pr) are still using Node.js 20. This inconsistency could lead to compatibility issues, especially since build artifacts from the build job (Node.js 22) are used in downstream jobs running Node.js 20.
Consider updating all jobs to use Node.js 22 for consistency:
# Update in all job sections:
- node-version: 20
+ node-version: 22Affected jobs that need the update:
- typecheck job (line ~120)
- lint job (line ~160)
- test-unit job (line ~190)
- build-release job (line ~310)
- release-pr job (line ~350)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify Node.js version consistency across all workflow files
# Check Node.js versions in all workflow files
echo "Node.js versions in workflow files:"
rg "node-version: \d+" .github/workflows/
Length of output: 847
|
in general we want to run at least some tests on the lowest node version nuxt supports, to avoid unintended regressions (for example, if using new features of node) |
Then will I target for node 18, 20 and 22 ? |
|
let's aim to run fixtures on node 18, and everything else on node 22. sound good? |
danielroe
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❤️
|
Thanks @danielroe . |
Since node 22 is stable, I have updated node version to 22.
Summary by CodeRabbit
New Features
Bug Fixes
GITHUB_TOKENin certain workflows.Chores