Skip to content

Conversation

@ZakaryCode
Copy link
Contributor

@ZakaryCode ZakaryCode commented Sep 19, 2025

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

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

  • 错误修复 (Bugfix) issue: fix #

这个 PR 涉及以下平台:

  • 鸿蒙(Harmony)

Summary by CodeRabbit

  • Bug Fixes
    • 修复构建产物中资源扩展名处理不当,避免同名不同后缀资源被覆盖,提升加载稳定性。
    • 扩大页面与依赖的匹配范围,修复部分目录下文件未编译导致的页面/组件缺失问题。
  • Improvements
    • 优化 Harmony 平台环境注入与配置获取流程,提升构建一致性与可靠性。

@coderabbitai
Copy link

coderabbitai bot commented Sep 19, 2025

Walkthrough

此次变更包含三处:1) 将 Harmony 注入环境的 Vite 插件导出由接收 platform 参数改为使用 this 绑定;2) Harmony 资源命名与磁盘路径策略调整,扩展名仅入磁盘路径不入公开 URL;3) 页面编译过滤范围扩展为 sourceDir 下的所有文件。

Changes

Cohort / File(s) Change summary
Harmony 插件签名调整
packages/taro-platform-harmony-cpp/src/program/vite/inject-env.ts
导出函数由 function (platform: Harmony) 改为 function (this: Harmony);通过 that = this 获取 getConfig()context.runnerUtils,其余逻辑不变。
Harmony 资源与页面处理
packages/taro-vite-runner/src/harmony/asset.ts, packages/taro-vite-runner/src/harmony/page.ts
资源:拆分扩展名,磁盘路径包含扩展名,公开 URL 不含扩展名;页面:createFilterWithCompileOptions 的 include 扩展为 ${sourceDir}/** 与原有的 node_modules taro 匹配并存。

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Dev as Vite (build/serve)
  participant P as inject-env 插件
  participant H as Harmony 实例(this)
  Dev->>+P: 初始化/调用插件钩子
  P->>+H: this.getConfig()
  H-->>-P: 返回配置(含 businessId)
  P->>H: 访问 this.context.runnerUtils
  P-->>-Dev: 返回注入后的处理结果
  note over P,H: 插件从参数读取 改为从 this 读取
Loading
sequenceDiagram
  autonumber
  participant U as 调用方
  participant A as asset.ts
  U->>+A: 请求生成资源引用(相对路径)
  A->>A: 提取扩展名 ext
  A->>A: 规范化名称(去扩展名)
  A->>A: 生成磁盘路径: base/media/<规范名><ext>
  A->>A: 生成公开URL: resource://base/media/<规范名>
  A-->>-U: {resourcePath 含扩展名, URL 不含扩展名}
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

  • yoturg
  • Single-Dancer

Poem

小兔敲键啪啦啪,
this 一指换天下。
资源扩展藏磁盘,
URL 清爽不带挂。
过滤网再铺大,
代码跃动春风下。 🐇✨

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 (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed 标题 "feat(harmony): 修复鸿蒙 assets 文件后缀 & 修复样式 filter 规则" 简洁且直接反映 PR 的两个主要修复点(assets 后缀处理和样式 filter 规则),与提供的变更摘要和 PR 目标一致,便于团队快速识别变更范围和目的。标题使用了合适的 scope 和类型,语言明确,没有冗余信息。
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/harmony-watch

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


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.

@ZakaryCode ZakaryCode added the T-harmony Target - 编译到 Harmony label Sep 19, 2025
@ZakaryCode ZakaryCode added this to 4.x Sep 19, 2025
@ZakaryCode ZakaryCode added this to the 4.1.7 milestone Sep 19, 2025
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 (3)
packages/taro-vite-runner/src/harmony/asset.ts (2)

286-288: 二进制资源被按 UTF‑8 写入,会损坏图片/音频等文件

此处 content 为 Buffer,但强制指定 { encoding: 'utf-8' } 会破坏二进制内容。

请移除编码参数:

-  fs.writeFileSync(resourcePath, content, {
-    encoding: 'utf-8',
-  })
+  fs.writeFileSync(resourcePath, content)

290-292: 缓存值与返回值不一致,导致重复加载时导出文件系统路径而非 resource:// URL

cache.set(id, resourcePath) 会让下次命中缓存返回磁盘路径,随后 load 导出成为绝对文件路径,破坏运行时预期。

修正为缓存并返回统一的 resource:// URL:

-  cache.set(id, resourcePath)
-  return 'resource://base/media/' + resourceName
+  const publicUrl = 'resource://base/media/' + resourceName
+  cache.set(id, publicUrl)
+  return publicUrl
packages/taro-platform-harmony-cpp/src/program/vite/inject-env.ts (1)

21-23: 潜在空引用崩溃:compiler 可能为空时 .concat 调用会抛错

compiler?.frameworkExts.concat(...)compiler?.frameworkExts 可能为 undefined,随后对 concat 的调用将抛异常。

建议健壮化合并逻辑:

-      const exts = Array.from(new Set(compiler?.frameworkExts.concat(SCRIPT_EXT)))
+      const exts = Array.from(new Set([...(compiler?.frameworkExts ?? []), ...SCRIPT_EXT]))
🧹 Nitpick comments (5)
packages/taro-vite-runner/src/harmony/page.ts (1)

25-25: Windows 路径匹配风险:对 include 模式中的 sourceDir 做路径归一化

在 Windows 下,直接拼接 ${sourceDir}/** 可能因反斜杠导致 picomatch 匹配失败。建议利用已引入的 escapePath 归一化后再拼接。

应用如下补丁:

-  const filter = createFilterWithCompileOptions(taroConfig.compile, [`${sourceDir}/**`, /(?<=node_modules[\\/]).*taro/], [])
+  const filter = createFilterWithCompileOptions(
+    taroConfig.compile,
+    [`${escapePath(sourceDir)}/**`, /(?<=node_modules[\\/]).*taro/],
+    []
+  )
packages/taro-vite-runner/src/harmony/asset.ts (2)

282-285: 资源名规范化存在两个小问题:字符类范围与移除扩展名方式

  • 使用 [^A-z0-9] 会把 [\]^_\`` 等非法字符也当作“字母”,建议改为 [^A-Za-z0-9_]`。
  • 通过 .replace(ext, '') 可能误删路径中间与扩展名同名的片段,建议仅删除末尾扩展名。

应用更稳妥的生成方式:

-  const ext = path.extname(file)
-  const resourceName = path.relative(appRoot, file).replace(ext, '').replace(/^[\\/]+/, '').replace(/[^A-z0-9]+/g, '_')
+  const ext = path.extname(file)
+  const rel = path.relative(appRoot, file).replace(/^[\\/]+/, '')
+  const base = ext ? rel.slice(0, -ext.length) : rel
+  const resourceName = base.replace(/[^A-Za-z0-9_]+/g, '_')

282-292: 可选:避免同名冲突(不同子目录/不同扩展名)导致覆盖

扁平化并去除扩展名会增加重名概率。可在冲突时追加短哈希后缀。

示例补丁(仅在发生冲突时追加 8 位 hash):

-  const resourcePath = path.join(escapePath(outputRoot), '..', 'resources/base/media', `${resourceName}${ext}`)
+  const mediaDir = path.join(escapePath(outputRoot), '..', 'resources/base/media')
+  let finalName = resourceName
+  let resourcePath = path.join(mediaDir, `${finalName}${ext}`)
+  if (fs.existsSync(resourcePath)) {
+    finalName = `${resourceName}_${getHash(rel).slice(0, 8)}`
+    resourcePath = path.join(mediaDir, `${finalName}${ext}`)
+  }
...
-  const publicUrl = 'resource://base/media/' + resourceName
+  const publicUrl = 'resource://base/media/' + finalName
packages/taro-platform-harmony-cpp/src/program/vite/inject-env.ts (2)

70-77: 使用 Vite 插件上下文的 warn 接口,而非 console.warn

在插件中调用 console.warn 无法纳入构建日志体系。应使用 pluginContext.warn(...)

应用如下补丁:

-                        console.warn(`Error: 使用定位相关 API(${path.node.callee.name}) 时,需要配置 defineConstants.LOCATION_APIKEY 为 businessId.`)
+                        pluginContext.warn(`使用定位相关 API(${path.node.callee.name}) 时,需要配置 defineConstants.LOCATION_APIKEY 为 businessId。`)
...
-                              console.warn(`Error: 使用定位相关 API(${path.node.property.name}) 时,需要配置 defineConstants.LOCATION_APIKEY 为 businessId.`)
+                              pluginContext.warn(`使用定位相关 API(${path.node.property.name}) 时,需要配置 defineConstants.LOCATION_APIKEY 为 businessId。`)

Also applies to: 95-101


47-56: 命名遮蔽可读性较差:局部变量 imported 与外层 imported 同名

const { imported } = spec 会遮蔽上层的 imported 布尔变量,读者易混淆。

小改名即可:

-                      const { imported } = spec
-                      const propertyName = t.isIdentifier(imported) ? imported.name : imported.value
+                      const { imported: importedToken } = spec
+                      const propertyName = t.isIdentifier(importedToken) ? importedToken.name : importedToken.value
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9ac4b8b and 4c7edc4.

📒 Files selected for processing (3)
  • packages/taro-platform-harmony-cpp/src/program/vite/inject-env.ts (1 hunks)
  • packages/taro-vite-runner/src/harmony/asset.ts (1 hunks)
  • packages/taro-vite-runner/src/harmony/page.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: ianzone
PR: NervJS/taro#18150
File: packages/taro-platform-harmony-hybrid/package.json:43-45
Timestamp: 2025-09-05T18:40:45.775Z
Learning: 在 tarojs/plugin-platform-harmony-hybrid 包中,tarojs/components-library-react、tarojs/components-library-solid 和 tarojs/components-library-vue3 必须作为直接依赖(dependencies)而不能作为 peer 依赖,因为插件源码中有对这些包的直接引用,包括 componentAdapter* getter 方法和 webpack 别名配置。
🧬 Code graph analysis (2)
packages/taro-vite-runner/src/harmony/asset.ts (2)
packages/taro-vite-runner/src/utils/index.ts (1)
  • escapePath (215-217)
packages/taro-platform-harmony-cpp/scripts/constant.ts (1)
  • outputRoot (15-15)
packages/taro-vite-runner/src/harmony/page.ts (1)
packages/taro-vite-runner/src/utils/createFilter.ts (1)
  • createFilterWithCompileOptions (79-99)
⏰ 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). (5)
  • GitHub Check: Build Rust Binding / stable - x86_64-pc-windows-msvc
  • GitHub Check: Build Rust Binding / stable - x86_64-apple-darwin
  • GitHub Check: Build Rust Binding / stable - x86_64-unknown-linux-gnu
  • GitHub Check: Build Rust Binding / stable - aarch64-apple-darwin
  • GitHub Check: Build Rust WASM / stable - wasm32-wasi
🔇 Additional comments (2)
packages/taro-platform-harmony-cpp/src/program/vite/inject-env.ts (1)

8-13: 确认:inject-env 的 this 绑定已正确使用(无需修改)

packages/taro-platform-harmony-cpp/src/program/index.ts 的 modifyPageConfig 中通过 injectEnv.call(this, opt) 调用(第116–119行),已为导出函数的 this 参数提供正确绑定。

packages/taro-vite-runner/src/harmony/asset.ts (1)

284-285: 确认 outputRoot 是否为 HAP 输出根 — 相对回退 '..' 可能出错

仓库检索仅在 packages/taro-vite-runner/src/harmony/asset.ts:284 发现该写法(const resourcePath = path.join(escapePath(outputRoot), '..', 'resources/base/media', ...));若 outputRoot 已经是 HAP 的输出根,向上退一层会把资源写到错误位置。建议核验 outputRoot 的来源/定义(配置或 constant),并改为基于 Harmony 打包根计算资源目录或显式构建目标路径(不要依赖通用的 '..' 回退),或在代码中对 outputRoot 的预期结构加断言/注释以避免误写。

@codecov
Copy link

codecov bot commented Sep 19, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 55.05%. Comparing base (17f1844) to head (b09d0ee).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main   #18376   +/-   ##
=======================================
  Coverage   55.05%   55.05%           
=======================================
  Files         416      416           
  Lines       21560    21560           
  Branches     5284     5284           
=======================================
  Hits        11870    11870           
+ Misses       8167     8077   -90     
- Partials     1523     1613   +90     
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.
see 49 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.

@ZakaryCode ZakaryCode merged commit 9ef8aaf into main Sep 19, 2025
39 checks passed
@ZakaryCode ZakaryCode deleted the feat/harmony-watch branch September 19, 2025 09:45
@github-project-automation github-project-automation bot moved this to Done in 4.x Sep 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-harmony Target - 编译到 Harmony

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants