revert(harmony): 还原 inject 插件对平台基类的错误引用#18326
Conversation
Walkthrough将 Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
packages/taro-platform-harmony-cpp/src/program/vite/inject-env.ts (4)
12-12: 对 businessId 去引号没问题，顺手 trim 更稳妥某些配置可能带空白符，建议在
replace前先trim，避免注入空格。- const businessId = that.getConfig().defineConstants?.LOCATION_APIKEY?.replace(/^['"]|['"]$/g, '') + const businessId = that.getConfig().defineConstants?.LOCATION_APIKEY?.trim()?.replace(/^['"]|['"]$/g, '')
18-20: 对 runnerUtils 的解构建议加防御并给出明确错误在极端/初始化顺序异常场景下，
that.context或runnerUtils可能为空，直接解构会抛错。建议先判空并通过 Vite 上下文报错，定位更清晰。- const { runnerUtils } = that.context - const { getViteHarmonyCompilerContext } = runnerUtils + const runnerUtils = that.context?.runnerUtils + if (!runnerUtils) { + pluginContext.error('[taro:harmony] runnerUtils 不可用：无法获取编译上下文') + return null + } + const { getViteHarmonyCompilerContext } = runnerUtils
70-79: DRY：抽取注入 businessId 的重复逻辑，降低维护风险
CallExpression与MemberExpression分支的注入代码基本重复。建议提取成局部助手函数，后续调整（如变更 key 名或校验策略）不易跑偏。应用于本段的局部替换示例：
- if (!hasBusinessId) { - if (!businessId) { - console.warn(`Error: 使用定位相关 API(${path.node.callee.name}) 时，需要配置 defineConstants.LOCATION_APIKEY 为 businessId.`) - return - } - args.properties.push( - t.objectProperty(t.identifier('businessId'), t.stringLiteral(businessId)) - ) - } + ensureBusinessId(args, path.node.callee.name, t)- if (!hasBusinessId) { - if (!businessId) { - console.warn(`Error: 使用定位相关 API(${path.node.property.name}) 时，需要配置 defineConstants.LOCATION_APIKEY 为 businessId.`) - return - } - args.properties.push( - t.objectProperty(t.identifier('businessId'), t.stringLiteral(businessId)) - ) - } + ensureBusinessId(args, path.node.property.name, t)在文件较前位置（与
businessId同作用域）新增助手函数：function ensureBusinessId(args: BabelCore.types.ObjectExpression, apiName: string, t: typeof BabelCore.types) { const hasBusinessId = args.properties.some( prop => !t.isSpreadElement(prop) && t.isIdentifier((prop as any).key) && (prop as any).key.name === 'businessId' ) if (!hasBusinessId) { if (!businessId) { console.warn(`Error: 使用定位相关 API(${apiName}) 时，需要配置 defineConstants.LOCATION_APIKEY 为 businessId.`) return } args.properties.push(t.objectProperty(t.identifier('businessId'), t.stringLiteral(businessId))) } }Also applies to: 94-102
8-9: 命名微调建议：that → platform，可读性更强
that = this没问题，但用platform更直接表达语义，易于维护。- const that = this + const platform = this（同步更新后续
that.*引用）
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/taro-platform-harmony-cpp/src/program/vite/inject-env.ts(1 hunks)
⏰ 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). (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 (1)
packages/taro-platform-harmony-cpp/src/program/vite/inject-env.ts (1)
8-9: inject-env 已正确使用.call(this, opt)绑定 this，无需调整
| const { runnerUtils } = that.context | ||
| const { getViteHarmonyCompilerContext } = runnerUtils | ||
| const compiler = getViteHarmonyCompilerContext(pluginContext) | ||
| const exts = Array.from(new Set(compiler?.frameworkExts.concat(SCRIPT_EXT))) |
There was a problem hiding this comment.
潜在崩溃：compiler?.frameworkExts 可能为 undefined，直接 concat 会抛异常
当 compiler 为 undefined 或未提供 frameworkExts 时，undefined.concat(...) 会直接中断构建。建议使用空数组回退。
- const exts = Array.from(new Set(compiler?.frameworkExts.concat(SCRIPT_EXT)))
+ const exts = Array.from(new Set([...(compiler?.frameworkExts ?? []), ...SCRIPT_EXT]))📝 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 exts = Array.from(new Set(compiler?.frameworkExts.concat(SCRIPT_EXT))) | |
| const exts = Array.from(new Set([...(compiler?.frameworkExts ?? []), ...SCRIPT_EXT])) |
🤖 Prompt for AI Agents
In packages/taro-platform-harmony-cpp/src/program/vite/inject-env.ts around line
21, using compiler?.frameworkExts.concat(SCRIPT_EXT) can throw if compiler or
frameworkExts is undefined; change to concatenate against a safe default (e.g.
use (compiler?.frameworkExts ?? []) or an empty array fallback) so concat always
operates on an array and avoid build crash.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #18326 +/- ##
=======================================
Coverage 55.05% 55.05%
=======================================
Files 416 416
Lines 21560 21560
Branches 5285 5300 +15
=======================================
Hits 11870 11870
- Misses 8034 8037 +3
+ Partials 1656 1653 -3
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
2 participants
这个 PR 做了什么？ (简要描述所做更改)
还原 inject 插件对平台基类的错误引用
这个 PR 是什么类型？ (至少选择一个)
这个 PR 涉及以下平台：
Summary by CodeRabbit
新特性
重构
影响