-
Notifications
You must be signed in to change notification settings - Fork 4.9k
chore: node22 #18578
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
base: main
Are you sure you want to change the base?
chore: node22 #18578
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Walkthrough更新多个 GitHub Actions 工作流将 Node 版本切换到 22,统一部分 runner 为 *-latest;调整 nodejs.yml 的矩阵与包含/排除规则,macOS 目标改为 aarch64,Ubuntu 同时包含 gnu 与 musl 目标;在 Stencil 测试配置中新增无沙箱浏览器启动参数。 Changes
Estimated code review effort🎯 3 (中等) | ⏱️ ~20 分钟
Possibly related PRs
Suggested reviewers
诗歌
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/nodejs.yml (1)
149-261: 建议重构:大量代码重复需要整合
nodejs-testing-newjob与原有的nodejs-testingjob几乎完全相同(约110行重复代码),仅在矩阵配置和第191行的安装命令有差异。这违反了DRY原则,增加了维护成本。根据PR描述,旧的
nodejs-testing最终会被移除,建议考虑以下方案:
- 短期:直接合并两个矩阵到一个job中(更新
node-version和host,调整exclude/include规则)- 或 使用GitHub Actions的reusable workflows来消除重复
这样可以避免未来维护两份相同代码的问题。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/nodejs.yml(1 hunks)
🔇 Additional comments (1)
.github/workflows/nodejs.yml (1)
156-165: 澄清矩阵排除规则的设计意图新job的矩阵配置中,Node 22被排除在所有非ubuntu平台上(macos-13、macos-latest、windows-latest),这意味着:
- Node 22:仅在ubuntu-latest上测试
- Node 24:在所有平台上测试
请确认这是否为故意设计(例如Node 22在其他平台上还不稳定),还是需要扩展Node 22的测试覆盖范围。
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #18578 +/- ##
==========================================
- Coverage 52.63% 51.83% -0.81%
==========================================
Files 463 545 +82
Lines 25108 28282 +3174
Branches 6629 7332 +703
==========================================
+ Hits 13215 14659 +1444
- Misses 9726 11331 +1605
- Partials 2167 2292 +125
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
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.
Pull request overview
This PR modernizes the CI/CD pipeline by updating Node.js versions and build infrastructure to support newer platforms while dropping legacy Intel Mac support.
Key Changes:
- Updates CI testing from Node.js 18.x/20.x to Node.js 22/24
- Migrates macOS builds from Intel (x86_64) to ARM (aarch64) architecture exclusively
- Adds browser sandbox flags for improved CI test stability
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
.github/workflows/nodejs.yml |
Updates Node.js test matrix to versions 22 and 24; switches macOS runner to latest (ARM); changes macOS target to aarch64-apple-darwin; simplifies pnpm install command |
.github/workflows/build-rust-binding.yml |
Updates to Node.js 22; removes Intel Mac (x86_64-apple-darwin) build target; migrates all hosts to latest versions; retains only ARM Mac builds |
packages/taro-components/scripts/stencil/stencil.config.ts |
Adds browser args for sandboxed CI environments; improves comment formatting with proper spacing |
Comments suppressed due to low confidence (1)
.github/workflows/build-rust-binding.yml:38
- [nitpick] The removal of the
x86_64-apple-darwinbuild target (Intel Mac) means the project will no longer build native bindings for Intel-based Macs, only for ARM-based Macs (aarch64-apple-darwin).
While Apple Silicon is now the standard, there are still users on Intel-based Macs who may need these bindings. Ensure this is an intentional decision and that either:
- Intel Mac users are no longer supported, or
- Rosetta 2 translation is acceptable for Intel Mac users (ARM binaries can run on Intel Macs via Rosetta, but with performance overhead)
Consider documenting this architecture support change in release notes if this represents a change in platform support policy.
settings:
- host: windows-latest
build: pnpm build:binding:release
target: x86_64-pc-windows-msvc
- host: ubuntu-latest
target: x86_64-unknown-linux-gnu
docker: ghcr.io/napi-rs/napi-rs/nodejs-rust:lts-debian
build: |-
set -e &&
npm install -g corepack@0.31.0 &&
pnpm build:binding:release --target x86_64-unknown-linux-gnu &&
strip crates/native_binding/*.node
- host: ubuntu-latest
target: x86_64-unknown-linux-musl
docker: ghcr.io/napi-rs/napi-rs/nodejs-rust:lts-alpine
build: set -e && pnpm build:binding:release && strip crates/native_binding/*.node
- host: macos-latest
target: aarch64-apple-darwin
build: |
pnpm build:binding:release --target aarch64-apple-darwin
strip -x crates/native_binding/*.node
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
macOS-13 已弃用,请及时更新分支保护规则! @luckyadam @tutuxxx |
@yoyo837 兄弟有更新权限吗?现在所有macOS-13的CI都跑不了了 |
我没有,我给群里吆喝一下吧。 |
先撤回 #18649? |
|
按理说,node 目前应该最低的 LTS 是20,为什么只留22、24呢? |
这个PR初衷也是想把node版本更新了 |
|
Taro 目前以稳定为主,如果不是必须,最好不动。 |
|
反正18必须弃用了,依赖也陆续不支持18了,20在5个月后也将弃用,还不如从现在起紧跟头两个LTS |
如果现在没有人手处理24可能的问题的话,那就保留20,等5个月后弃用了再更24 |
|
@yoyo837 咋说?保留20还是24? |
能不动则不动 |
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: 0
🧹 Nitpick comments (1)
.github/workflows/sync-components-types.yml (1)
44-48: 注意 ts-node 兼容性问题(第 44 行注释)注释表明当前同步脚本与 ts-node 在 Node 20+ 存在兼容问题,已标记为待修复。升级至 Node 22 后,需要确认
pnpm --filter=@tarojs/components run sync:types脚本(第 57 行)的执行状态。建议在此 PR 合并后跟进 ts-node 兼容性修复,或在下一次 CI 运行中监控该步骤是否有报错。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/nodejs.yml(1 hunks).github/workflows/publish.yml(2 hunks).github/workflows/sync-components-types.yml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/nodejs.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build Rust Binding / stable - x86_64-pc-windows-msvc
- GitHub Check: Build Rust Binding / stable - x86_64-unknown-linux-gnu
🔇 Additional comments (4)
.github/workflows/publish.yml (3)
101-104: 更新 Node.js 版本至 22,步骤名称改为通用标签此变更与 PR 目标一致,将 Node 版本从 18 升级到 22 以支持最新 LTS。步骤名称改为 "Setup Node" 更加通用,便于未来灵活调整版本。
178-178: 正确的拼写修正步骤名称从 "Checkout Harmony Commond Line Tools" 改为 "Checkout Harmony Command Line Tools",正确修复了拼写错误。
171-171: The-rflag is unnecessary for this workflow;pnpm installworks equivalently in the Taro monorepo.In Taro's pnpm workspace,
pnpm installexecuted at the repository root automatically performs a recursive installation across all workspace packages because pnpm'srecursiveInstalldefaults to true. This meanspnpm install --frozen-lockfileandpnpm -r install --frozen-lockfileare functionally equivalent when run from the root directory..github/workflows/sync-components-types.yml (1)
45-48: Node.js 版本升级至 22,与发布工作流一致此更新与
.github/workflows/publish.yml中的 Node 版本升级保持一致,符合 PR 目标。步骤名称也改为通用标签。
留了20,删了24 |
|
不更新的话,合上去也没有用吧? |
合上去至少可以让CI跑起来,只是会有pending checks |
这个 PR 做了什么? (简要描述所做更改)
更新 node 版本为 node22
更新 macOS 至 latest arm 架构,actions/runner-images#13046
这个 PR 是什么类型? (至少选择一个)
这个 PR 涉及以下平台:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.