-
Notifications
You must be signed in to change notification settings - Fork 4.9k
fix(rn-runner): 为 spawn 启用 shell #18478
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
fix(rn-runner): 为 spawn 启用 shell #18478
Conversation
问题:
在 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
Walkthrough在 React Native 构建流程的 spawn 调用中添加了 Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 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: 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对象中的参数(如port、bundleOutput、sourcemapOutput等)包含恶意内容,可能存在命令注入风险。虽然这些参数通常来自配置文件,但仍建议添加验证。建议验证关键参数:
// 在 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
📒 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
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ 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
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
问题:
在 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 是什么类型? (至少选择一个)
这个 PR 涉及以下平台:
Summary by CodeRabbit
发布说明