-
Notifications
You must be signed in to change notification settings - Fork 4.9k
fix(ascf): 调整ascf.config.json的生成逻辑 #18726
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
Conversation
Walkthrough在 Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)📚 Learning: 2025-08-25T22:16:50.118ZApplied to files:
⏰ 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). (3)
🔇 Additional comments (3)
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: 1
🤖 Fix all issues with AI agents
In @packages/taro-platform-ascf/src/program.ts:
- Line 15: The field initializer projectConfigJson accesses
this.config.projectConfigName before the class constructor runs, causing a
timing bug; move the initialization into the AscfApp (or TaroPlatform)
constructor after this.config = config is assigned, or replace the initializer
with a getter that returns this.config?.projectConfigName || PROJECT_CONFIG_NAME
so evaluation is delayed; update references to projectConfigJson to use the new
constructor-assigned property or the getter to avoid accessing this.config in a
field initializer.
🧹 Nitpick comments (1)
packages/taro-platform-ascf/src/program.ts (1)
45-51: 方法签名兼容性已验证,代码实现正确。确认了以下事实:
父类
TaroPlatformBase的generateProjectConfig()方法签名为protected generateProjectConfig (src: string, dist = 'project.config.json'),确实接受两个参数(源配置名和目标配置名)。框架调用流程通过
this.projectConfigJson传递源配置名,该值已通过this.config.projectConfigName || PROJECT_CONFIG_NAME赋值,参数传递正确。当前重写方法的设计是有意的:允许源配置文件名可自定义(通过
this.config.projectConfigName),但输出始终生成ascf.config.json,符合注释说明"ascf不需要生成project.config.json,这里override,内部调用时直接生成ascf.config.json"的设计意图。建议:可在注释中更明确说明
src参数是源配置文件名,而输出文件名固定为ascf.config.json,以避免开发者对projectConfigJson字段允许自定义与方法输出名称固定之间的关系产生疑惑。
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/taro-platform-ascf/src/program.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-25T22:16:50.118Z
Learnt from: ianzone
Repo: NervJS/taro PR: 18150
File: packages/babel-plugin-transform-taroapi/tests/__mocks__/h5-definition.json:2356-2356
Timestamp: 2025-08-25T22:16:50.118Z
Learning: The file `packages/babel-plugin-transform-taroapi/tests/__mocks__/h5-definition.json` is auto-generated by the post-build script `packages/taro-platform-h5/scripts/post-build.mjs`, which copies content from `taro-platform-h5/dist/definition.json`. This file should not be manually edited.
Applied to files:
packages/taro-platform-ascf/src/program.ts
⏰ 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). (2)
- GitHub Check: Build Rust Binding / stable - x86_64-unknown-linux-gnu
- GitHub Check: Build Rust Binding / stable - x86_64-pc-windows-msvc
🔇 Additional comments (1)
packages/taro-platform-ascf/src/program.ts (1)
9-9: 常量提取遵循最佳实践。将硬编码字符串提取为模块级常量
PROJECT_CONFIG_NAME符合DRY原则,与上方的PACKAGE_NAME一致。
| platform = 'ascf' | ||
| globalObject = 'has' | ||
| projectConfigJson: string = this.config.projectConfigName || 'ascf.config.json' | ||
| projectConfigJson: string = this.config.projectConfigName || PROJECT_CONFIG_NAME |
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.
🧩 Analysis chain
🏁 Script executed:
# Find the TaroPlatformBase class definition
fd -e ts ".*" | xargs rg -l "class TaroPlatformBase" | head -5Repository: NervJS/taro
Length of output: 109
🏁 Script executed:
# Get more context about TaroPlatformBase initialization
rg "class TaroPlatformBase" -A 30 | head -50Repository: NervJS/taro
Length of output: 2895
🏁 Script executed:
# Check where this.config is set in the codebase
rg "this\.config\s*=" | grep -E "(TaroPlatformBase|program\.ts)" | head -20Repository: NervJS/taro
Length of output: 37
🏁 Script executed:
# Find TaroPlatform class definition (parent of TaroPlatformBase)
rg "class TaroPlatform" -A 30 | head -60Repository: NervJS/taro
Length of output: 5286
🏁 Script executed:
# Look at the actual file being reviewed
cat -n packages/taro-platform-ascf/src/program.ts | head -30Repository: NervJS/taro
Length of output: 1082
🏁 Script executed:
# Check if this.config is initialized in constructor body vs field initializer
rg "this\.config\s*=" -B 5 -A 2 | grep -A 7 "constructor\|TaroPlatform" | head -40Repository: NervJS/taro
Length of output: 2653
修复字段初始化器中对 this.config 的访问时序问题。
字段初始化器在父类构造函数执行前运行。第 15 行的初始化器试图访问 this.config.projectConfigName,但 this.config 只在 TaroPlatform 的构造函数体中被赋值(this.config = config),此时尚未执行。建议将此初始化逻辑移至 AscfApp 的构造函数中或使用 getter 方法来延迟求值。
🤖 Prompt for AI Agents
In @packages/taro-platform-ascf/src/program.ts at line 15, The field initializer
projectConfigJson accesses this.config.projectConfigName before the class
constructor runs, causing a timing bug; move the initialization into the AscfApp
(or TaroPlatform) constructor after this.config = config is assigned, or replace
the initializer with a getter that returns this.config?.projectConfigName ||
PROJECT_CONFIG_NAME so evaluation is delayed; update references to
projectConfigJson to use the new constructor-assigned property or the getter to
avoid accessing this.config in a field initializer.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #18726 +/- ##
=======================================
Coverage 56.32% 56.32%
=======================================
Files 447 447
Lines 23345 23345
Branches 5748 5750 +2
=======================================
Hits 13149 13149
+ Misses 8373 8366 -7
- Partials 1823 1830 +7
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
这个 PR 做了什么? (简要描述所做更改)
这个 PR 是什么类型? (至少选择一个)
这个 PR 涉及以下平台:
Summary by CodeRabbit
发行说明
✏️ Tip: You can customize this high-level summary in your review settings.