Skip to content

Conversation

@namehu
Copy link
Contributor

@namehu namehu commented Oct 16, 2025

问题:
在 Windows(PowerShell/CMD)环境中, rn-runner 通过 child_process.spawn 启动 metro/打包时未启用 shell, 可能导致命令解析或路径解析失败,出现开发服务器或打包无法启动的问题。

解决:

影响范围:
全平台:Wind、MacOS测试通过

验证:
在 Windows 环境验证 RN 开发与打包流程可正常运行,日志输出符合预期。

Closes #18180, #17744, #16938

这个 PR 做了什么? (简要描述所做更改)

在 src/index.ts 的两处 spawn 调用设置 { stdio: 'inherit', shell: true }

这个 PR 是什么类型? (至少选择一个)

  • 错误修复 (Bugfix) issue: fix #
  • 新功能 (Feature)
  • 代码重构 (Refactor)
  • TypeScript 类型定义修改 (Types)
  • 文档修改 (Docs)
  • 代码风格更新 (Code style update)
  • 构建优化 (Chore)
  • 其他,请描述 (Other, please describe):

这个 PR 涉及以下平台:

  • 所有平台
  • Web 端(H5)
  • 移动端(React-Native)
  • 鸿蒙(Harmony)
  • 鸿蒙容器(Harmony Hybrid)
  • ASCF 元服务
  • 快应用(QuickApp)
  • 所有小程序
  • 微信小程序
  • 企业微信小程序
  • 京东小程序
  • 百度小程序
  • 支付宝小程序
  • 支付宝 IOT 小程序
  • 钉钉小程序
  • QQ 小程序
  • 飞书小程序
  • 快手小程序
  • 头条小程序

Summary by CodeRabbit

发布说明

  • Chores
    • 优化了 React Native 构建流程的执行配置,改进了启动和打包过程的兼容性。

问题:
在 Windows(PowerShell/CMD)环境中, rn-runner 通过 child_process.spawn 启动 metro/打包时未启用 shell,
可能导致命令解析或路径解析失败,出现开发服务器或打包无法启动的问题。

解决:
在 src/index.ts 的两处 spawn 调用设置 { stdio: 'inherit', shell: true }

影响范围:
全平台:Wind、MacOS测试通过

验证:
在 Windows 环境验证 RN 开发与打包流程可正常运行,日志输出符合预期。

Closes NervJS#18180, NervJS#17744, NervJS#16938
@coderabbitai
Copy link

coderabbitai bot commented Oct 16, 2025

Walkthrough

在 React Native 构建流程的 spawn 调用中添加了 shell: true 选项,涉及启动服务和打包两种模式(监听模式和非监听模式),保持 stdio: 'inherit' 不变,无其他控制流改动。

Changes

Cohort / 文件 变更摘要
RN Runner 启动配置
packages/taro-rn-runner/src/index.ts
在两处 spawn 调用中添加 shell: true 选项,覆盖 React Native 启动和打包流程的监听模式和非监听模式路径

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 长耳朵竖起来,Windows 上的 spawn 不再怨
Shell 真言一句加,EINVAL 错误化烟雾
跨平台之路更通坦,构建流水向前涌
Taro 一家亲,RN 速运转!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed 拉取请求标题“fix(rn-runner): 为 spawn 启用 shell”简洁明了地概述了主要变更,即为 rn-runner 中的 spawn 调用开启 shell 支持,准确反映了代码修改的核心目的,并避免了冗余或含糊表述。
Linked Issues Check ✅ Passed 该 PR 在两个 spawn 调用中新增 shell:true 选项,正是针对 #18180 中在 Windows 上出现 spawn EINVAL 报错的问题所需的代码改动,满足了关联 Issue 的核心编码需求。
Out of Scope Changes Check ✅ Passed 本次提交仅在 rn-runner 的 spawn 调用中添加了 shell:true,未涉及其他无关功能或模块的修改,所有变更均与解决 Issue 提出的问题直接相关。
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a 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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
packages/taro-rn-runner/src/index.ts (2)

64-71: 监听 spawn 进程事件以捕获异步错误。

当前代码未捕获 spawn 返回的 ChildProcess 对象,也未监听其事件。由于 spawn 是异步的,try-catch 只能捕获同步错误(如参数验证失败),无法捕获进程启动后的错误(如命令不存在、权限问题等)。这可能导致静默失败。

建议应用以下修复:

     try {
-      spawn(npxCmd, [
+      const child = spawn(npxCmd, [
         'react-native',
         'start',
         '--custom-log-reporter-path',
         '@tarojs/rn-supporter/TerminalReporter'
       ].concat(cliParams), {
         stdio: 'inherit', shell: true
       })
+      child.on('error', (err) => {
+        console.error('Failed to start React Native dev server:', err)
+        onFinish(err)
+      })
+      child.on('exit', (code, signal) => {
+        if (code !== 0 && code !== null) {
+          const error = new Error(`Dev server exited with code ${code}`)
+          console.error(error)
+          onFinish(error)
+        }
+      })
       if (config.qr) {

108-119: 监听 spawn 进程事件以捕获打包错误。

与开发服务器的 spawn 调用类似,这里也未监听 ChildProcess 事件。打包过程中的错误(如磁盘空间不足、metro 配置错误等)可能无法被正确捕获和报告,导致构建静默失败。

建议应用以下修复:

     try {
-      spawn(npxCmd, [
+      const child = spawn(npxCmd, [
         'react-native',
         'bundle',
         '--platform',
         config.deviceType,
         '--dev',
         'false',
         '--entry-file',
         'index.js'
       ].concat(cliParams), {
         stdio: 'inherit', shell: true
       })
+      child.on('error', (err) => {
+        console.error('Failed to bundle React Native app:', err)
+        onFinish(err)
+      })
+      child.on('exit', (code, signal) => {
+        if (code !== 0 && code !== null) {
+          const error = new Error(`Bundle process exited with code ${code}`)
+          console.error(error)
+          onFinish(error)
+        }
+      })
       if (config.qr) {
🧹 Nitpick comments (1)
packages/taro-rn-runner/src/index.ts (1)

64-71: 验证命令参数以防止潜在的命令注入风险。

启用 shell: true 后,如果 config 对象中的参数(如 portbundleOutputsourcemapOutput 等)包含恶意内容,可能存在命令注入风险。虽然这些参数通常来自配置文件,但仍建议添加验证。

建议验证关键参数:

// 在 build 函数开始处添加参数验证
function validateConfig(config: any) {
  // 验证端口号
  if (config.port && (isNaN(parseInt(config.port)) || parseInt(config.port) < 0 || parseInt(config.port) > 65535)) {
    throw new Error(`Invalid port: ${config.port}`)
  }
  // 验证路径参数,防止路径遍历
  const pathParams = ['bundleOutput', 'sourcemapOutput', 'assetsDest']
  pathParams.forEach(param => {
    if (config[param] && (config[param].includes('..') || /[;<>|&$`]/.test(config[param]))) {
      throw new Error(`Invalid path parameter ${param}: ${config[param]}`)
    }
  })
}

export default async function build (_appPath: string, config: any): Promise<any> {
  validateConfig(config)
  // ... 其余代码

另外,可以考虑使用 spawn 的数组形式传递参数,而不是依赖 shell 解析,但这需要更复杂的跨平台处理。

Also applies to: 108-119

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1414e3d and d290259.

📒 Files selected for processing (1)
  • packages/taro-rn-runner/src/index.ts (2 hunks)
🔇 Additional comments (1)
packages/taro-rn-runner/src/index.ts (1)

70-70: 核心修复正确:shell: true 解决 Windows spawn 问题。

添加 shell: true 选项正确解决了 Windows 上的 spawn EINVAL 错误。在 Windows 上,npx.cmd 是批处理文件,必须通过 shell 执行。此修改在 Windows 和 macOS 上均已验证,符合跨平台要求。

Also applies to: 118-118

@Single-Dancer Single-Dancer added this to the 4.1.8 milestone Oct 16, 2025
@codecov
Copy link

codecov bot commented Oct 16, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 55.97%. Comparing base (7420fda) to head (fdc406d).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##             main   #18478    +/-   ##
========================================
  Coverage   55.97%   55.97%            
========================================
  Files         416      416            
  Lines       21560    21560            
  Branches     5267     5298    +31     
========================================
  Hits        12069    12069            
- Misses       7904     8010   +106     
+ Partials     1587     1481   -106     
Flag Coverage Δ
taro-cli 72.85% <ø> (ø)
taro-runtime 59.87% <ø> (ø)
taro-web 53.13% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
packages/taro-rn-runner/src/index.ts 0.00% <ø> (ø)

... and 51 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

This was referenced Oct 18, 2025
@Single-Dancer Single-Dancer merged commit 66cef71 into NervJS:main Oct 22, 2025
94 of 101 checks passed
This was referenced Oct 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Build:RN 报错

2 participants