taro 4.x主包适配效能插件 #17492
taro 4.x主包适配效能插件 #17492
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #17492 +/- ##
=======================================
Coverage 57.86% 57.86%
=======================================
Files 468 468
Lines 26458 26458
Branches 5826 5838 +12
=======================================
Hits 15309 15309
+ Misses 9669 9666 -3
- Partials 1480 1483 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
zhengyanan1
force-pushed
the
feat/performance-measure
branch
from
31c2eca to
a258931
Compare
Walkthrough此次变更添加了一个新的 postinstall 脚本及对应的 Changes
Sequence Diagram(s)sequenceDiagram
participant npm
participant postinstallJS as "postinstall.js"
participant TaroSite as "https://taro.jd.com/"
participant System
npm->>postinstallJS: 执行 postinstall 脚本
postinstallJS->>TaroSite: 发起 GET 请求 (超时 5000ms)
alt 请求成功
postinstallJS->>System: 执行命令添加性能报告插件
else 请求失败
postinstallJS->>postinstallJS: 记录错误信息
end
sequenceDiagram
participant Caller as "Runner/CLI"
participant ViteModifier as "modifyViteConfig"
participant PluginSystem
Caller->>ViteModifier: 调用 modifyViteConfig(commonConfig, {componentConfig}, viteCompilerContext)
ViteModifier->>PluginSystem: applyPlugins(viteConfig, data, viteCompilerContext)
PluginSystem-->>ViteModifier: 返回修改后的配置
ViteModifier-->>Caller: 返回更新后的 Vite 配置
Poem
✨ Finishing Touches
🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
packages/taro-cli/package.json (1)
11-11: 添加了 postinstall 脚本这个新增的 postinstall 脚本会在包安装后执行
postinstall.js文件。需要注意 postinstall 脚本会影响安装性能，并且如果在该脚本中执行网络请求可能会有安全和性能方面的考虑。建议确保 postinstall.js 中的逻辑具有合适的超时处理和错误捕获机制，以避免在网络不可用时阻塞整个安装流程。
packages/taro-vite-runner/src/index.h5.ts (1)
47-53: 更新了 modifyViteConfig 方法调用参数与 harmony.ts 文件中的变更类似，这里也在
modifyViteConfig调用中添加了第三个参数viteCompilerContext。这种一致性的变更表明这是一项有计划的重构。这些类似的变更可能适合提取一个辅助方法来统一处理
modifyViteConfig的调用，减少重复代码。-taroConfig.modifyViteConfig?.( - commonConfig, - { - componentConfig - }, - viteCompilerContext -) +callModifyViteConfig(taroConfig, commonConfig, { componentConfig }, viteCompilerContext)packages/taro-vite-runner/src/h5/pipeline.ts (1)
30-38: 添加了构建完成回调机制新增的 closeBundle 方法实现了构建完成时的回调机制，这符合 PR 中提到的性能插件适配需求。这种方式可以让插件在构建完成时执行自定义逻辑。
不过，建议考虑增加对潜在错误的处理：
closeBundle () { const onBuildFinish = taroConfig.onBuildFinish if (isFunction(onBuildFinish)) { - onBuildFinish({ - error: null, - stats: {}, - isWatch: taroConfig.isWatch - }) + try { + onBuildFinish({ + error: null, + stats: {}, + isWatch: taroConfig.isWatch + }) + } catch (error) { + console.error('执行 onBuildFinish 回调时发生错误:', error) + } } }packages/taro-service/src/utils/types.ts (1)
154-154: 扩展了 modifyViteConfig 方法签名modifyViteConfig 方法现在接收额外的 viteCompilerContext 参数，这与 packages/taro-cli/src/presets/commands/build.ts 中的更新保持一致，允许插件访问更多上下文信息来修改 Vite 配置。
但是，使用
any类型会导致类型安全性降低。建议使用更具体的类型：- modifyViteConfig: (fn: (args: { viteConfig: any, data?: IModifyChainData, viteCompilerContext: any }) => void) => void + modifyViteConfig: (fn: (args: { viteConfig: any, data?: IModifyChainData, viteCompilerContext: ViteCompilerContext }) => void) => void其中 ViteCompilerContext 应该是一个定义明确的接口类型。如果这个类型已经在项目中定义，请导入并使用它；如果没有，建议创建一个适当的类型定义。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
packages/taro-cli/package.json(2 hunks)packages/taro-cli/postinstall.js(1 hunks)packages/taro-cli/src/cli.ts(1 hunks)packages/taro-cli/src/presets/commands/build.ts(1 hunks)packages/taro-service/src/Config.ts(3 hunks)packages/taro-service/src/utils/index.ts(2 hunks)packages/taro-service/src/utils/types.ts(1 hunks)packages/taro-vite-runner/src/h5/pipeline.ts(2 hunks)packages/taro-vite-runner/src/index.h5.ts(1 hunks)packages/taro-vite-runner/src/index.harmony.ts(1 hunks)packages/taro-vite-runner/src/index.mini.ts(1 hunks)
🧰 Additional context used
🧬 Code Definitions (2)
packages/taro-service/src/Config.ts (1)
packages/taro-service/src/utils/index.ts (1)
filterGlobalConfig(121-135)
packages/taro-service/src/utils/types.ts (1)
packages/taro-cli/src/presets/commands/build.ts (1)
fn(52-220)
🪛 Biome (1.9.4)
packages/taro-service/src/utils/index.ts
[error] 129-129: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Build Rust Binding / stable - aarch64-apple-darwin
- GitHub Check: Build Rust Binding / stable - x86_64-apple-darwin
- GitHub Check: Build Rust Binding / stable - x86_64-pc-windows-msvc
- GitHub Check: Build Rust Binding / stable - x86_64-pc-windows-msvc
🔇 Additional comments (13)
packages/taro-cli/package.json (1)
27-27: 将 postinstall.js 添加到 files 数组中这是正确的做法，确保新添加的 postinstall.js 文件会被包含在 npm 包中。
packages/taro-vite-runner/src/index.harmony.ts (1)
35-41:✅ Verification successful
更新了 modifyViteConfig 方法调用参数
对
modifyViteConfig的调用增加了第三个参数viteCompilerContext，以提供更多上下文信息给配置修改过程。这与其他文件中的类似更改保持一致。
🏁 Script executed:
#!/bin/bash # 检查所有使用 modifyViteConfig 的地方是否已更新为使用新的参数列表 rg -A 2 "modifyViteConfig" --glob "*.ts" | grep -v "viteCompilerContext"Length of output: 3372
更新 modifyViteConfig 调用参数 —— 已成功添加第三个参数
经过验证，在文件
packages/taro-vite-runner/src/index.harmony.ts中对modifyViteConfig的调用已正确新增了第三个参数viteCompilerContext，以提供更多上下文信息。这一调整符合本次变更的初衷，并与其他实现类似功能的文件中相应修改保持了一致性。
- 正在更新的调用已在
packages/taro-vite-runner/src/index.harmony.ts中添加了第三个参数viteCompilerContext。- 验证脚本确认该文件的调用参数已更新，而其他文件中未作相同更改的部分（如测试文件及部分平台入口）依据各自业务需求可保持原状。
packages/taro-cli/src/cli.ts (1)
10-10:❓ Verification inconclusive
从 DISABLE_GLOBAL_CONFIG_COMMANDS 中移除了 'build' 命令
这个变更允许 build 命令使用全局配置，而之前是禁用的。这可能会改变构建行为，使其能够使用全局配置中定义的选项。
需要确认这个变更是否会导致现有构建配置的行为发生变化，特别是对于那些从未预期使用全局配置的项目。
🏁 Script executed:
#!/bin/bash # 检查全局配置相关的代码变更 rg -A 5 "DISABLE_GLOBAL_CONFIG_COMMANDS|initGlobalConfig|filterGlobalConfig" --glob "*.ts"Length of output: 3173
确认全局配置对 build 命令的影响
经过代码库检索，发现当前逻辑中：
- 在
packages/taro-cli/src/cli.ts中，通过判断DISABLE_GLOBAL_CONFIG_COMMANDS来决定是否禁用全局配置，而现在对应数组中不再包含'build'命令；- 在
packages/taro-service/src/Config.ts中，初始化全局配置时会调用filterGlobalConfig处理各命令传入的配置参数。也就是说，当执行
build命令时，由于不再被禁用，程序会加载并处理全局配置。这意味着对于预期不使用全局配置的项目，可能会出现构建行为上的变化。请确认这是否符合设计预期，如有必要，可考虑在文档中提示或增加兼容性处理。packages/taro-service/src/Config.ts (3)
18-18: 导入了用于命令过滤的工具函数添加了对
filterGlobalConfig函数的导入，该函数用于根据命令过滤全局配置中的插件。
56-56: 在初始化全局配置时传入了命令参数将
configEnv.command作为参数传递给initGlobalConfig方法，支持基于命令的配置过滤。
72-80:✅ Verification successful
更新了initGlobalConfig方法签名并应用过滤
方法签名更新为接受一个可选的command参数，并使用filterGlobalConfig函数过滤全局配置中的插件。
请验证
filterGlobalConfig函数的过滤逻辑是否可能导致某些全局配置在某些命令下被意外排除：
🏁 Script executed:
#!/bin/bash # 检查是否有依赖该全局配置的其他文件 rg -A 5 "initialGlobalConfig" --type ts packages/Length of output: 3347
确认过滤逻辑无明显问题
经过对全局配置在各处引用情况的检索，目前没有发现因使用
filterGlobalConfig而导致必要全局配置被意外剔除的情况。引用该配置的地方（例如Kernel.ts中对initialGlobalConfig.plugins和initialGlobalConfig.presets的处理）均能正确获取所需信息。建议在确保该函数内部实现覆盖所有预期命令场景后，再进行进一步验证，但就目前来看不存在问题。packages/taro-vite-runner/src/index.mini.ts (1)
40-46: 更新了modifyViteConfig方法调用，增加了viteCompilerContext参数为
modifyViteConfig方法调用添加了第三个参数viteCompilerContext，使其与其他文件中的变更保持一致。这个更改使方法调用与其签名更新保持同步，确保了Vite配置修改过程中可以访问编译器上下文信息。
packages/taro-cli/src/presets/commands/build.ts (3)
148-148: 更新了modifyViteConfig方法签名，增加viteCompilerContext参数方法签名更新为包含第三个参数
viteCompilerContext，使其与调用方保持一致。
155-155: 在MODIFY_VITE_CONFIG钩子选项中添加了viteCompilerContext将
viteCompilerContext添加到传递给插件的选项中，使插件能够访问编译器上下文信息。
206-215:✅ Verification successful
确认onBuildFinish钩子是否需要更新
onBuildFinish钩子函数在构建完成时被调用，但与本次其他修改不同，它没有接收viteCompilerContext参数。请确认是否需要为此钩子也添加相应参数。
🏁 Script executed:
#!/bin/bash # 检查其他钩子函数是否需要viteCompilerContext参数 rg -A 3 "viteCompilerContext" packages/ --type tsLength of output: 80494
确认 onBuildFinish 钩子参数处理（无需更新）
经过对比项目中其他钩子（尤其是 taro-vite-runner 里的实现）以及在 taro-cli 的 onBuildFinish 钩子当前传入的参数，可以确认该钩子设计上仅需要 error、stats 和 isWatch 三个参数，不需要额外传入 viteCompilerContext。现有实现与项目中各平台的职责划分保持一致，因此无需对 onBuildFinish 钩子进行更新。
packages/taro-vite-runner/src/h5/pipeline.ts (2)
2-2: 添加了 isFunction 工具函数导入从 @tarojs/shared 包导入 isFunction 工具函数，用于后续对函数类型的检查，这是一个良好的实践。
10-10: 优化了 taroConfig 的提取位置将 taroConfig 的解构移到函数顶部，提高了代码的可读性和可维护性，使得配置对象在整个函数范围内都可用。
packages/taro-service/src/utils/index.ts (1)
9-9: 更新导入类型声明添加了 IProjectConfig 类型导入，为新增的 filterGlobalConfig 函数提供类型支持，这是一个良好的 TypeScript 实践。
packages/taro-cli/postinstall.js
Outdated
| const { exec } = require('child_process') | ||
| const axios = require('axios') | ||
|
|
||
| axios.get('https://taro.jd.com/', { timeout: 5000 }) | ||
| .then(() => { | ||
| exec('taro global-config add-plugin @jdtaro/plugin-build-report-performance@latest --registry http://registry.m.jd.com', (error, _stdout, _stderr) => { | ||
| if (error) { | ||
| console.error(`install performance plugin error: ${error}`) | ||
| } | ||
| }) | ||
| }) | ||
| .catch(() => { | ||
| }) |
There was a problem hiding this comment.
检查执行的外部命令和网络请求的安全性
这段代码在包安装后自动发送网络请求并执行外部命令，存在几个值得关注的问题：
- 代码向外部域名发送请求，但没有明确告知用户目的
- 在请求成功后，自动安装一个特定的性能插件
- 使用了特定的注册表URL (http://registry.m.jd.com)
- 请求失败时没有任何错误处理逻辑（空catch块）
建议修改为：
const { exec } = require('child_process')
const axios = require('axios')
+// 检查是否在京东内部环境
+function isJDEnvironment() {
+ // 实现环境检测逻辑
+ return true // 暂时返回true作为示例
+}
-axios.get('https://taro.jd.com/', { timeout: 5000 })
+// 仅在特定环境下执行插件安装
+if (isJDEnvironment()) {
+ axios.get('https://taro.jd.com/', { timeout: 5000 })
.then(() => {
exec('taro global-config add-plugin @jdtaro/plugin-build-report-performance@latest --registry http://registry.m.jd.com', (error, _stdout, _stderr) => {
if (error) {
console.error(`install performance plugin error: ${error}`)
+ } else {
+ console.log('性能插件安装成功')
}
})
})
.catch((error) => {
+ console.error(`检查Taro网站可用性失败: ${error.message}`)
})
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const { exec } = require('child_process') | |
| const axios = require('axios') | |
| axios.get('https://taro.jd.com/', { timeout: 5000 }) | |
| .then(() => { | |
| exec('taro global-config add-plugin @jdtaro/plugin-build-report-performance@latest --registry http://registry.m.jd.com', (error, _stdout, _stderr) => { | |
| if (error) { | |
| console.error(`install performance plugin error: ${error}`) | |
| } | |
| }) | |
| }) | |
| .catch(() => { | |
| }) | |
| const { exec } = require('child_process') | |
| const axios = require('axios') | |
| // 检查是否在京东内部环境 | |
| function isJDEnvironment() { | |
| // 实现环境检测逻辑 | |
| return true // 暂时返回true作为示例 | |
| } | |
| // 仅在特定环境下执行插件安装 | |
| if (isJDEnvironment()) { | |
| axios.get('https://taro.jd.com/', { timeout: 5000 }) | |
| .then(() => { | |
| exec('taro global-config add-plugin @jdtaro/plugin-build-report-performance@latest --registry http://registry.m.jd.com', (error, _stdout, _stderr) => { | |
| if (error) { | |
| console.error(`install performance plugin error: ${error}`) | |
| } else { | |
| console.log('性能插件安装成功') | |
| } | |
| }) | |
| }) | |
| .catch((error) => { | |
| console.error(`检查Taro网站可用性失败: ${error.message}`) | |
| }) | |
| } |
zhengyanan1
force-pushed
the
feat/performance-measure
branch
from
a258931 to
f4de2e5
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/taro-service/src/utils/index.ts (1)
121-135: 新增函数中存在对输入参数的直接修改
filterGlobalConfig函数直接修改了输入参数globalConfig，这可能导致意外的副作用。建议创建一个深拷贝以避免直接修改输入参数。export function filterGlobalConfig (globalConfig: IProjectConfig, command: string) { if (!command) { return globalConfig } - const config = globalConfig + const config = { ...globalConfig } const RelatedPluginTag = `@jdtaro/plugin-${command}` if (config.plugins?.length) { config.plugins = config.plugins.filter(pluginName => { return pluginName.includes(RelatedPluginTag) }) } return config }🧰 Tools
🪛 Biome (1.9.4)
[error] 128-129: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/taro-cli/postinstall.js(1 hunks)packages/taro-service/src/Config.ts(3 hunks)packages/taro-service/src/utils/index.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/taro-cli/postinstall.js
- packages/taro-service/src/Config.ts
🧰 Additional context used
🪛 Biome (1.9.4)
packages/taro-service/src/utils/index.ts
[error] 128-129: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build Rust Binding / stable - x86_64-pc-windows-msvc
- GitHub Check: Build Rust Binding / stable - x86_64-pc-windows-msvc
🔇 Additional comments (1)
packages/taro-service/src/utils/index.ts (1)
128-128: 正确使用了可选链操作符使用
config.plugins?.length进行非空检查是一个很好的实践，避免了潜在的运行时错误。🧰 Tools
🪛 Biome (1.9.4)
[error] 128-129: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
| if (config.plugins?.length) { | ||
| config.plugins = config.plugins.filter(pluginName => { | ||
| return pluginName.includes(RelatedPluginTag) | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
使用includes可能导致匹配不精确
使用includes进行插件名称匹配可能会导致意外的匹配结果。例如，如果存在一个名为@jdtaro/plugin-buildextension的插件，它会被错误地包含在build命令的过滤结果中。
export function filterGlobalConfig (globalConfig: IProjectConfig, command: string) {
if (!command) {
return globalConfig
}
const config = globalConfig
const RelatedPluginTag = `@jdtaro/plugin-${command}`
if (config.plugins?.length) {
config.plugins = config.plugins.filter(pluginName => {
- return pluginName.includes(RelatedPluginTag)
+ // 使用更精确的匹配方式，比如完全匹配或者正则表达式
+ return pluginName === RelatedPluginTag ||
+ pluginName.startsWith(`${RelatedPluginTag}@`) ||
+ pluginName.startsWith(`${RelatedPluginTag}/`)
})
}
return config
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (config.plugins?.length) { | |
| config.plugins = config.plugins.filter(pluginName => { | |
| return pluginName.includes(RelatedPluginTag) | |
| }) | |
| export function filterGlobalConfig (globalConfig: IProjectConfig, command: string) { | |
| if (!command) { | |
| return globalConfig | |
| } | |
| const config = globalConfig | |
| const RelatedPluginTag = `@jdtaro/plugin-${command}` | |
| if (config.plugins?.length) { | |
| config.plugins = config.plugins.filter(pluginName => { | |
| // 使用更精确的匹配方式，比如完全匹配或者正则表达式 | |
| return pluginName === RelatedPluginTag || | |
| pluginName.startsWith(`${RelatedPluginTag}@`) || | |
| pluginName.startsWith(`${RelatedPluginTag}/`) | |
| }) | |
| } | |
| return config | |
| } |
🧰 Tools
🪛 Biome (1.9.4)
[error] 128-129: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
zhengyanan1
force-pushed
the
feat/performance-measure
branch
3 times, most recently
from
c3066b8 to
5b4493b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/taro-cli/postinstall.js (1)
1-14:⚠️ Potential issue这个 postinstall 脚本存在多个安全性和用户体验问题
这段代码在包安装后自动发送网络请求并执行外部命令，存在几个重要问题：
- 代码向外部域名发送请求，但没有明确告知用户目的
- 在请求成功后，自动安装一个特定的性能插件，没有用户确认步骤
- 使用了特定的注册表URL (http://registry.m.jd.com)，而且是非HTTPS的
- 请求失败时日志消息("cli postinstall success too")与实际情况不符
- 使用了相对路径 './bin/taro'，可能在某些环境中无法正确解析
建议进行如下改进：
- 增加环境检测，仅在特定环境下执行这些操作
- 添加详细的日志说明脚本的行为和目的
- 使用HTTPS注册表URL
- 修复失败情况下的错误日志
- 考虑使用更可靠的路径解析方式
const { exec } = require('child_process') const axios = require('axios') +// 检查是否在特定环境中 +function isTargetEnvironment() { + // 实现环境检测逻辑 + return process.env.ENABLE_PERFORMANCE_PLUGIN === 'true' || process.env.JD_ENVIRONMENT === 'true' +} + +// 只在特定环境下执行插件安装 +if (isTargetEnvironment()) { + console.log('正在检查并安装性能插件...') axios.get('https://taro.jd.com/', { timeout: 5000 }) .then(() => { - exec('./bin/taro global-config add-plugin @jdtaro/plugin-build-report-performance@latest --registry http://registry.m.jd.com', (error, _stdout, _stderr) => { + // 使用更可靠的路径解析方式 + const taroPath = require.resolve('@tarojs/cli/bin/taro') + exec(`node "${taroPath}" global-config add-plugin @jdtaro/plugin-build-report-performance@latest --registry https://registry.m.jd.com`, (error, _stdout, _stderr) => { if (error) { console.error(`install performance plugin error: ${error}`) + } else { + console.log('性能插件安装成功') } }) }) .catch((error) => { - console.log('cli postinstall success too') + console.error(`检查Taro网站可用性失败: ${error.message || '未知错误'}`) }) +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/taro-cli/postinstall.js(1 hunks)packages/taro-service/src/utils/index.ts(2 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
packages/taro-service/src/utils/index.ts (1)
packages/taro/types/compile/config/project.d.ts (1)
IProjectConfig(210-323)
🪛 Biome (1.9.4)
packages/taro-service/src/utils/index.ts
[error] 128-129: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Build Rust Binding / stable - aarch64-apple-darwin
- GitHub Check: Build Rust Binding / stable - x86_64-apple-darwin
- GitHub Check: Build Rust Binding / stable - x86_64-pc-windows-msvc
- GitHub Check: Build Rust Binding / stable - aarch64-apple-darwin
- GitHub Check: Build Rust Binding / stable - x86_64-unknown-linux-gnu
🔇 Additional comments (1)
packages/taro-service/src/utils/index.ts (1)
9-9: 已更新导入以包含 IProjectConfig 类型导入语句已经正确更新，以包含新函数所需的
IProjectConfig类型。
| export function filterGlobalConfig (globalConfig: IProjectConfig, command: string) { | ||
| if (!command) { | ||
| return globalConfig | ||
| } | ||
| const config = globalConfig | ||
|
|
||
| const RelatedPluginTag = `@jdtaro/plugin-${command}-` | ||
| if (config.plugins?.length) { | ||
| config.plugins = config.plugins.filter(pluginName => { | ||
| return pluginName.includes(RelatedPluginTag) | ||
| }) | ||
| } | ||
|
|
||
| return config | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
过滤全局配置的实现需要优化
新增的 filterGlobalConfig 函数根据命令过滤插件列表，但有以下几点需要改进：
- 插件名称匹配不够精确 - 使用
includes可能会导致意外匹配 - 函数直接修改了输入参数
globalConfig，可能导致意外的副作用 - 缺少函数文档注释，不清楚函数的具体用途和预期行为
RelatedPluginTag变量名末尾带有连字符-，是否为有意为之？
建议按照以下方式优化实现：
+/**
+ * 根据命令名过滤全局配置中的插件
+ * @param globalConfig 全局配置对象
+ * @param command 命令名称
+ * @returns 过滤后的配置对象
+ */
export function filterGlobalConfig (globalConfig: IProjectConfig, command: string) {
if (!command) {
return globalConfig
}
- const config = globalConfig
+ // 创建配置对象的浅拷贝，避免修改原始对象
+ const config = { ...globalConfig }
- const RelatedPluginTag = `@jdtaro/plugin-${command}-`
+ const RelatedPluginTag = `@jdtaro/plugin-${command}`
if (config.plugins?.length) {
config.plugins = config.plugins.filter(pluginName => {
- return pluginName.includes(RelatedPluginTag)
+ // 使用更精确的匹配方式
+ return typeof pluginName === 'string' && (
+ pluginName === RelatedPluginTag ||
+ pluginName.startsWith(`${RelatedPluginTag}@`) ||
+ pluginName.startsWith(`${RelatedPluginTag}/`) ||
+ pluginName.startsWith(`${RelatedPluginTag}-`)
+ )
})
}
return config
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function filterGlobalConfig (globalConfig: IProjectConfig, command: string) { | |
| if (!command) { | |
| return globalConfig | |
| } | |
| const config = globalConfig | |
| const RelatedPluginTag = `@jdtaro/plugin-${command}-` | |
| if (config.plugins?.length) { | |
| config.plugins = config.plugins.filter(pluginName => { | |
| return pluginName.includes(RelatedPluginTag) | |
| }) | |
| } | |
| return config | |
| } | |
| /** | |
| * 根据命令名过滤全局配置中的插件 | |
| * @param globalConfig 全局配置对象 | |
| * @param command 命令名称 | |
| * @returns 过滤后的配置对象 | |
| */ | |
| export function filterGlobalConfig (globalConfig: IProjectConfig, command: string) { | |
| if (!command) { | |
| return globalConfig | |
| } | |
| // 创建配置对象的浅拷贝，避免修改原始对象 | |
| const config = { ...globalConfig } | |
| const RelatedPluginTag = `@jdtaro/plugin-${command}` | |
| if (config.plugins?.length) { | |
| config.plugins = config.plugins.filter(pluginName => { | |
| // 使用更精确的匹配方式 | |
| return typeof pluginName === 'string' && ( | |
| pluginName === RelatedPluginTag || | |
| pluginName.startsWith(`${RelatedPluginTag}@`) || | |
| pluginName.startsWith(`${RelatedPluginTag}/`) || | |
| pluginName.startsWith(`${RelatedPluginTag}-`) | |
| ) | |
| }) | |
| } | |
| return config | |
| } |
🧰 Tools
🪛 Biome (1.9.4)
[error] 128-129: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
packages/taro-cli/postinstall.js (2)
4-14:⚠️ Potential issue自动网络请求和命令执行存在安全风险
这段代码在包安装后自动发送网络请求并执行外部命令安装插件，存在以下几个问题：
- 未经用户同意就发送网络请求到外部域名
- 使用了特定的企业内部注册表URL (http://registry.m.jd.com)
- 执行命令使用相对路径
./bin/taro，可能在某些安装环境中不可用- 请求失败时没有任何错误处理逻辑
- 成功日志在命令执行完成前就打印出来，可能产生误导
建议进行以下修改：
-axios.get('https://taro.jd.com/', { timeout: 5000 }) - .then(() => { - exec('./bin/taro global-config add-plugin @jdtaro/plugin-build-report-performance@latest --registry http://registry.m.jd.com', (error, _stdout, _stderr) => { - if (error) { - console.error(`install performance plugin error: ${error}`) - } - }) - console.log('cli postinstall success') - }) - .catch(() => { - }) +// 仅在特定环境下执行插件安装 +if (isInternalEnvironment()) { + console.log('正在检查Taro性能插件配置...') + axios.get('https://taro.jd.com/', { timeout: 5000 }) + .then(() => { + const taroPath = require.resolve('@tarojs/cli/bin/taro') + exec(`node "${taroPath}" global-config add-plugin @jdtaro/plugin-build-report-performance@latest --registry http://registry.m.jd.com`, (error, _stdout, _stderr) => { + if (error) { + console.error(`安装性能插件失败: ${error}`) + } else { + console.log('性能插件配置成功') + } + }) + }) + .catch((error) => { + console.error(`检查Taro网站可用性失败: ${error.message}`) + }) +} else { + console.log('跳过性能插件安装（非内部环境）') +}
1-3: 🛠️ Refactor suggestion引入了必要模块，但缺少安全检查
代码导入了执行外部命令和发送网络请求所需的模块，但未实现任何环境检测或安全验证逻辑。
建议添加环境检测函数，确保只在适当的环境中执行：
const { exec } = require('child_process') const axios = require('axios') +// 检查是否在内部环境 +function isInternalEnvironment() { + // 实现环境检测逻辑，例如检查特定环境变量 + return process.env.INTERNAL_ENV === 'true' +}
🧹 Nitpick comments (1)
packages/taro-cli/postinstall.js (1)
1-14: 考虑使用可配置的选项代替硬编码值代码中包含多个硬编码的值，如URL、超时时间、插件名称和注册表地址，这降低了代码的灵活性和可维护性。
建议使用配置文件或环境变量来提供这些值：
const { exec } = require('child_process') const axios = require('axios') +const fs = require('fs') +const path = require('path') +// 默认配置 +const DEFAULT_CONFIG = { + checkUrl: 'https://taro.jd.com/', + timeout: 5000, + pluginName: '@jdtaro/plugin-build-report-performance@latest', + registry: 'http://registry.m.jd.com' +} + +// 尝试加载自定义配置 +let config = DEFAULT_CONFIG +try { + const configPath = path.resolve(__dirname, './postinstall.config.js') + if (fs.existsSync(configPath)) { + const customConfig = require(configPath) + config = { ...DEFAULT_CONFIG, ...customConfig } + } +} catch (error) { + console.warn(`加载自定义配置失败: ${error.message}`) +} // 实现部分略...使用config对象中的值代替硬编码值
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/taro-cli/postinstall.js(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build Rust Binding / stable - x86_64-apple-darwin
zhengyanan1
force-pushed
the
feat/performance-measure
branch
from
dbf58e7 to
86b0ba8
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
packages/taro-service/src/utils/index.ts (1)
121-135: 🛠️ Refactor suggestion新增的过滤全局配置函数需要优化
新增的
filterGlobalConfig函数存在以下问题：
- 直接修改了输入参数
globalConfig，可能导致意外的副作用- 使用
includes进行插件名称匹配不够精确- 缺少函数文档注释
RelatedPluginTag变量名末尾带有连字符-，可能导致匹配问题建议按照以下方式优化实现：
+/** + * 根据命令名过滤全局配置中的插件 + * @param globalConfig 全局配置对象 + * @param command 命令名称 + * @returns 过滤后的配置对象 + */ export function filterGlobalConfig (globalConfig: IProjectConfig, command: string) { if (!command) { return globalConfig } - const config = globalConfig + // 创建配置对象的浅拷贝，避免修改原始对象 + const config = { ...globalConfig } - const RelatedPluginTag = `@jdtaro/plugin-${command}-` + const RelatedPluginTag = `@jdtaro/plugin-${command}` if (config.plugins?.length) { config.plugins = config.plugins.filter(pluginName => { - return pluginName.includes(RelatedPluginTag) + // 使用更精确的匹配方式 + return typeof pluginName === 'string' && ( + pluginName === RelatedPluginTag || + pluginName.startsWith(`${RelatedPluginTag}@`) || + pluginName.startsWith(`${RelatedPluginTag}/`) || + pluginName.startsWith(`${RelatedPluginTag}-`) + ) }) } return config }🧰 Tools
🪛 Biome (1.9.4)
[error] 128-129: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
packages/taro-cli/postinstall.js (1)
1-14: 🛠️ Refactor suggestion安全性与最佳实践问题
这段代码在执行 postinstall 时存在几个安全性和最佳实践的问题：
- 自动发起网络请求并安装插件，没有明确告知用户或提供选择机制
- 使用硬编码的域名和注册表URL
- 请求失败时没有任何错误处理逻辑（空catch块）
- 成功日志位置不正确，在命令执行完成前就输出了成功信息
- 使用相对路径
./bin/taro调用命令，这假设了特定的目录结构建议进行以下优化：
const { exec } = require('child_process') const axios = require('axios') +// 检查是否在特定环境 +function shouldInstallPlugin() { + // 实现环境检测逻辑，如检查特定环境变量 + return process.env.INSTALL_TARO_PERFORMANCE_PLUGIN === 'true' +} +// 使用环境变量或配置文件获取URL，而不是硬编码 +const TARO_WEBSITE = process.env.TARO_WEBSITE || 'https://taro.jd.com/' +const TARO_REGISTRY = process.env.TARO_REGISTRY || 'http://registry.m.jd.com' +const PLUGIN_PATH = '@jdtaro/plugin-build-report-performance@latest' +if (shouldInstallPlugin()) { -axios.get('https://taro.jd.com/', { timeout: 5000 }) + axios.get(TARO_WEBSITE, { timeout: 5000 }) .then(() => { - exec('./bin/taro global-config add-plugin @jdtaro/plugin-build-report-performance@latest --registry http://registry.m.jd.com', (error, _stdout, _stderr) => { + // 使用npm bin路径查找taro命令，避免依赖相对路径 + const taroCmd = 'npx taro' + exec(`${taroCmd} global-config add-plugin ${PLUGIN_PATH} --registry ${TARO_REGISTRY}`, (error, _stdout, _stderr) => { if (error) { console.error(`install performance plugin error: ${error}`) + } else { + console.log('性能插件安装成功') } }) - console.log('cli postinstall success') }) .catch(() => { + console.error('无法连接到Taro网站，跳过性能插件安装') }) +} else { + console.log('跳过性能插件安装（不在目标环境中）') +}此外，建议在
package.json中添加依赖："dependencies": { "axios": "^x.x.x" }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
packages/taro-cli/package.json(2 hunks)packages/taro-cli/postinstall.js(1 hunks)packages/taro-cli/src/cli.ts(1 hunks)packages/taro-cli/src/presets/commands/build.ts(1 hunks)packages/taro-service/src/Config.ts(3 hunks)packages/taro-service/src/utils/index.ts(2 hunks)packages/taro-service/src/utils/types.ts(1 hunks)packages/taro-vite-runner/src/h5/pipeline.ts(2 hunks)packages/taro-vite-runner/src/index.h5.ts(1 hunks)packages/taro-vite-runner/src/index.harmony.ts(1 hunks)packages/taro-vite-runner/src/index.mini.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (9)
- packages/taro-cli/src/cli.ts
- packages/taro-service/src/Config.ts
- packages/taro-vite-runner/src/index.harmony.ts
- packages/taro-service/src/utils/types.ts
- packages/taro-vite-runner/src/h5/pipeline.ts
- packages/taro-vite-runner/src/index.h5.ts
- packages/taro-cli/package.json
- packages/taro-cli/src/presets/commands/build.ts
- packages/taro-vite-runner/src/index.mini.ts
🧰 Additional context used
🧬 Code Definitions (1)
packages/taro-service/src/utils/index.ts (1)
packages/taro/types/compile/config/project.d.ts (1)
IProjectConfig(210-323)
🪛 Biome (1.9.4)
packages/taro-service/src/utils/index.ts
[error] 128-129: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build Rust Binding / stable - x86_64-pc-windows-msvc
- GitHub Check: Build Rust Binding / stable - x86_64-pc-windows-msvc
🔇 Additional comments (1)
packages/taro-service/src/utils/index.ts (1)
9-9: 导入声明更新正确导入声明已正确更新，添加了
IProjectConfig类型，以支持新增的filterGlobalConfig函数参数类型定义。
|
|
||
| axios.get('https://taro.jd.com/', { timeout: 5000 }) | ||
| .then(() => { | ||
| exec('./bin/taro global-config add-plugin @jdtaro/plugin-build-report-performance@latest --registry http://registry.m.jd.com', (error, _stdout, _stderr) => { |
There was a problem hiding this comment.
路径问题可能导致命令执行失败
当前命令使用了相对路径 ./bin/taro，这可能在不同的安装环境中导致路径解析错误。
建议使用 npx taro 或通过 Node.js 的 PATH 解析方式查找 taro 可执行文件：
- exec('./bin/taro global-config add-plugin @jdtaro/plugin-build-report-performance@latest --registry http://registry.m.jd.com', (error, _stdout, _stderr) => {
+ exec('npx taro global-config add-plugin @jdtaro/plugin-build-report-performance@latest --registry http://registry.m.jd.com', (error, _stdout, _stderr) => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| exec('./bin/taro global-config add-plugin @jdtaro/plugin-build-report-performance@latest --registry http://registry.m.jd.com', (error, _stdout, _stderr) => { | |
| exec('npx taro global-config add-plugin @jdtaro/plugin-build-report-performance@latest --registry http://registry.m.jd.com', (error, _stdout, _stderr) => { |
| .catch(() => { | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
缺少错误处理
当前代码在 Axios 请求失败时使用空的 catch 块，这会静默忽略所有错误，导致调试困难。
建议添加适当的错误处理：
.catch(() => {
+ console.error('无法连接到Taro网站，跳过性能插件安装')
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .catch(() => { | |
| }) | |
| .catch(() => { | |
| console.error('无法连接到Taro网站，跳过性能插件安装') | |
| }) |
这个 PR 做了什么? (简要描述所做更改)
适配效能插件
这个 PR 是什么类型? (至少选择一个)
这个 PR 涉及以下平台:
Summary by CodeRabbit
新功能
优化与重构