-
Notifications
You must be signed in to change notification settings - Fork 266
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
feat: 接入 harmony cpp 版本 #2646
feat: 接入 harmony cpp 版本 #2646
Conversation
Walkthrough本次更改涉及多个文件的修改,主要包括对 Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
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: 4
🧹 Outside diff range and nitpick comments (5)
scripts/harmony/clone-jdharmony.js (2)
6-6
: 命令行参数处理逻辑清晰,但可以进一步改进新增的命令行参数处理逻辑简洁有效,能够根据输入参数确定平台。这种方法提高了脚本的灵活性。
建议添加参数验证,以处理意外的输入:
if (args.length > 0 && !['cpp', 'harmony'].includes(args[0])) { console.error('无效的参数。请使用 "cpp" 或不传参数。'); process.exit(1); }Also applies to: 11-13
Line range hint
14-46
: 克隆过程保持稳健,但可以改进错误处理克隆过程的核心逻辑保持不变,正确地使用了更新后的变量。这保证了脚本的稳定性和灵活性。
建议增加错误处理来提高脚本的健壮性。例如:
async function cloneJdHarmony() { try { // ... existing code ... await git.clone(remote, ['-b', branch, '--depth', '1']) console.log(`Clone completed successfully.`) // ... rest of the function ... } catch (error) { console.error(`克隆失败: ${error.message}`) process.exit(1) } }这样可以更好地处理可能出现的错误,如网络问题或权限问题。
package.json (1)
68-68
: 检查dev:taro:jdharmony
脚本中的额外空格在
dev:taro:jdharmony
脚本中,clone:jdharmony
命令后添加了一个额外的空格。虽然这不会影响功能,但可能是无意的更改。建议检查并确认是否需要这个额外的空格:
- "dev:taro:jdharmony": "pnpm run clone:jdharmony && pnpm run update:taro:entry && pnpm --dir ./packages/nutui-taro-demo dev:jdharmony", + "dev:taro:jdharmony": "pnpm run clone:jdharmony && pnpm run update:taro:entry && pnpm --dir ./packages/nutui-taro-demo dev:jdharmony",packages/nutui-taro-demo/config/index.js (2)
62-62
: 变量命名建议:isHarmonycpp
为保持变量命名的一致性和可读性,建议将
isHarmonycpp
修改为isJdHarmonyCpp
,使其符合驼峰命名法,并与其他变量命名风格一致。
144-144
: 确认注释掉的配置是否仍需保留以下配置被注释掉:
// config: { selectorBlackList: ['nut-', 'demo', 'index', 'page'] },
请确认是否仍需保留该配置。如果不再需要,建议删除注释的代码,以保持代码的整洁。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
- .gitignore (1 hunks)
- .npmrc (0 hunks)
- package.json (1 hunks)
- packages/nutui-taro-demo/config/index.js (3 hunks)
- packages/nutui-taro-demo/package.json (3 hunks)
- scripts/harmony/clone-jdharmony.js (2 hunks)
💤 Files with no reviewable changes (1)
- .npmrc
✅ Files skipped from review due to trivial changes (1)
- .gitignore
🧰 Additional context used
🔇 Additional comments (10)
scripts/harmony/clone-jdharmony.js (3)
18-18
: 分支选择逻辑正确且简洁基于平台参数选择分支的逻辑实现得当。使用三元运算符使代码简洁明了,同时保持了良好的可读性。
22-22
: 日志输出更新恰当日志输出语句的更新很好地反映了新的平台变量,提供了更具体的信息。使用模板字符串提高了可读性。
Also applies to: 24-24
Line range hint
1-46
: 总体评价:脚本灵活性显著提升,仍有小幅改进空间本次更改成功地增强了脚本的灵活性,通过引入命令行参数支持不同的平台(jdharmony 和 jdharmonycpp)。主要改进包括:
- 动态平台和分支选择
- 更新的日志输出,提供更具体的信息
- 动态路径构建,适应不同平台
这些变更使脚本更加通用和可维护。然而,仍有一些小的改进空间:
- 添加参数验证
- 修正潜在的路径不一致问题
- 增强错误处理
总的来说,这些更改是积极的,提高了脚本的整体质量和可用性。建议在合并之前考虑实施提出的小改进。
packages/nutui-taro-demo/package.json (4)
25-25
: 新增的 jdharmonycpp 脚本新增了用于 jdharmonycpp 的开发脚本。这个添加符合 PR 的目标,即集成 Harmony C++ 版本。
Line range hint
1-120
: 总结评审意见
新增的 jdharmonycpp 脚本和 @jdtaro/plugin-platform-jdharmony-cpp 依赖符合 PR 的目标,支持了 Harmony C++ 版本的集成。
然而,大量 @tarojs 相关依赖从稳定版 4.0.2 降级到了 beta 版本 4.0.0-beta.138,这可能引入不稳定性和兼容性问题。
建议:
- 确认使用 @tarojs 4.0.0-beta.138 版本的必要性,评估其带来的潜在风险。
- 在合并前,在隔离环境中全面测试项目,确保与新版本的兼容性和稳定性。
- 考虑是否可以等待 @tarojs 4.0.0 正式版发布后再进行更新。
- 密切关注 @tarojs 的更新日志,了解 beta 版本中的重要变更。
请在合并前运行完整的测试套件,并验证所有功能在新版本下是否正常工作。同时,建议创建一个回滚计划,以防新版本在生产环境中出现问题。
64-80
:⚠️ Potential issue更新 @tarojs 依赖版本
多个 @tarojs 相关的依赖已更新到版本 4.0.0-beta.138。这是一个较大的版本变更,从稳定版 4.0.2 降级到了 beta 版本。
降级到 beta 版本可能会引入不稳定性或不兼容的更改。请确认是否有必要使用这个 beta 版本,以及它是否与项目的其他部分兼容。建议运行以下命令来检查潜在的兼容性问题:
#!/bin/bash # 检查 @tarojs 依赖的兼容性 npm ls @tarojs/components @tarojs/helper @tarojs/plugin-framework-react @tarojs/plugin-html @tarojs/plugin-platform-alipay @tarojs/plugin-platform-h5 @tarojs/plugin-platform-jd @tarojs/plugin-platform-qq @tarojs/plugin-platform-swan @tarojs/plugin-platform-tt @tarojs/plugin-platform-weapp @tarojs/react @tarojs/router @tarojs/runtime @tarojs/shared @tarojs/taro @tarojs/taro-h5 # 检查这些包的更新日志,查看是否有重大变更 for package in components helper plugin-framework-react plugin-html plugin-platform-alipay plugin-platform-h5 plugin-platform-jd plugin-platform-qq plugin-platform-swan plugin-platform-tt plugin-platform-weapp react router runtime shared taro taro-h5 do echo "Checking changelog for @tarojs/$package" npm view @tarojs/$package changelog done请评估使用 beta 版本的风险,并考虑是否应该等待稳定版本发布后再进行更新。
61-61
: 新增 @jdtaro/plugin-platform-jdharmony-cpp 依赖添加了 @jdtaro/plugin-platform-jdharmony-cpp 依赖,版本为 0.0.88-beta.0。这个新依赖与 PR 的目标一致,支持 Harmony C++ 版本的集成。
请确认此版本是否为最新的稳定版本,并验证其与项目其他依赖的兼容性。可以运行以下命令检查:
package.json (2)
Line range hint
2-2
: 版本更新至 1.0.9包版本已更新至 1.0.9。这个更新似乎与新增的 Harmony C++ 支持相关。
请确保在发布新版本之前更新 CHANGELOG.md 文件,记录新增的 Harmony C++ 支持功能。
运行以下脚本来验证 CHANGELOG.md 是否已更新:
#!/bin/bash # 描述:检查 CHANGELOG.md 是否包含版本 1.0.9 的更新记录 grep -n "## 1.0.9" CHANGELOG.md -A 10
69-69
: 新增dev:taro:jdharmonycpp
脚本新增了
dev:taro:jdharmonycpp
脚本,用于支持 Harmony C++ 版本的开发。这与 PR 的目标一致。请确保相关的
clone:jdharmony
脚本已更新,以支持新的cpp
参数。同时,验证packages/nutui-taro-demo
目录下是否已添加相应的dev:jdharmonycpp
命令。运行以下脚本来验证相关更改:
✅ Verification successful
验证通过
相关的
clone:jdharmony
脚本已支持cpp
参数,并且packages/nutui-taro-demo
目录下已添加dev:jdharmonycpp
命令。🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 描述:验证 clone:jdharmony 脚本和 nutui-taro-demo 包的更新 # 测试:检查 clone:jdharmony 脚本是否支持 cpp 参数 cat ./scripts/harmony/clone-jdharmony.js # 测试:检查 nutui-taro-demo 包是否包含 dev:jdharmonycpp 命令 cat ./packages/nutui-taro-demo/package.json | grep "dev:jdharmonycpp"Length of output: 1784
packages/nutui-taro-demo/config/index.js (1)
127-132
: 验证新增依赖的版本号添加了
ohPackage
配置,包含以下依赖:ohPackage: { dependencies: { '@jd-oh/taro_library': '2.0.70', '@jd-oh/taro_cpp_library': '0.0.88-beta.0' }, },请确保所添加的依赖版本号正确且稳定,避免引入潜在的兼容性问题。
const harmonyPath = path.join(temp, 'JDHarmony') | ||
const harmonyPathNew = path.join(temp, 'nutui-jdharmony') | ||
const harmonyPathNew = path.join(temp, `nutui-${platform}`) |
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.
路径修改正确,但存在潜在问题
新目录路径的修改正确地反映了所选平台,使用模板字符串构建路径的方法很恰当。
然而,第28行的 harmonyPath
仍然使用静态名称 'JDHarmony'。这可能导致在某些情况下找不到正确的目录。建议修改如下:
const harmonyPath = path.join(temp, `JDHarmony-${platform}`)
这样可以确保 harmonyPath
与克隆的仓库名称一致。
Also applies to: 42-42
"@tarojs/cli": "4.0.0-beta.138", | ||
"@tarojs/plugin-platform-harmony-ets": "4.0.0-beta.138", | ||
"@tarojs/vite-runner": "4.0.0-beta.138", | ||
"@tarojs/webpack5-runner": "4.0.0-beta.138", |
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.
if (process.env.TARO_ENV === 'jdharmony_cpp') { | ||
plugins = ['@jdtaro/plugin-platform-jdharmony-cpp'] | ||
} |
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.
🛠️ Refactor suggestion
建议统一 plugins
的赋值方式
在针对不同的 TARO_ENV
时,部分条件使用了直接赋值 plugins
的方式,例如:
if (process.env.TARO_ENV === 'jdharmony_cpp') {
plugins = ['@jdtaro/plugin-platform-jdharmony-cpp']
}
而其他条件则使用了 plugins.push()
方法,例如:
if (process.env.TARO_ENV === 'harmony') {
plugins.push('@tarojs/plugin-platform-harmony-ets')
}
为了代码的一致性和可读性,建议统一 plugins
的赋值方式,避免混淆。
// 将编译方式设置为使用 Vite 编译 | ||
compiler: { type: 'vite', vitePlugins: [injectScss()] }, | ||
// 【必填】鸿蒙主应用的绝对路径,例如: | ||
projectPath: path.resolve(process.cwd(), isHarmony ? '../nutui-harmony' : '../nutui-jdharmony'), | ||
projectPath: path.resolve(process.cwd(), isHarmony ? '../nutui-harmony' : isHarmonycpp ? '../nutui-jdharmonycpp' : '../nutui-jdharmony'), |
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.
检查 projectPath
的条件逻辑
在 projectPath
中,新增了对 isHarmonycpp
的判断:
projectPath: path.resolve(process.cwd(), isHarmony ? '../nutui-harmony' : isHarmonycpp ? '../nutui-jdharmonycpp' : '../nutui-jdharmony'),
请确认该逻辑满足预期,确保当 TARO_ENV
为 jdharmony_cpp
时,路径能够正确解析。
@@ -65,7 +65,8 @@ | |||
"dev:taro:h5:jmapp": "pnpm --dir ./packages/nutui-taro-demo dev:h5:jmapp", | |||
"dev:taro:alipay": "pnpm --dir ./packages/nutui-taro-demo dev:alipay", | |||
"dev:taro:harmony": "pnpm run update:taro:entry && pnpm --dir ./packages/nutui-taro-demo dev:harmony", | |||
"dev:taro:jdharmony": "pnpm run clone:jdharmony && pnpm run update:taro:entry && pnpm --dir ./packages/nutui-taro-demo dev:jdharmony", | |||
"dev:taro:jdharmony": "pnpm run clone:jdharmony && pnpm run update:taro:entry && pnpm --dir ./packages/nutui-taro-demo dev:jdharmony", |
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.
从 Taro 的规划看,这块可以删除,以 cpp 为准。
@@ -50,13 +49,17 @@ if (process.env.TARO_ENV === 'jdhybrid') { | |||
if (process.env.TARO_ENV === 'jdharmony') { | |||
plugins = ['@jdtaro/taro-platform-jdharmony'] |
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.
jdharmony 相关应该都可以删掉了
🤔 这个变动的性质是?
🔗 相关 Issue
💡 需求背景和解决方案
☑️ 请求合并前的自查清单
Summary by CodeRabbit
dev:jdharmonycpp
,支持jdharmony_cpp
平台。@jdtaro/plugin-platform-jdharmony-cpp
。.gitignore
文件,新增忽略条目。.npmrc
文件,影响 npm 和 Yarn 的配置。