-
Notifications
You must be signed in to change notification settings - Fork 4.9k
fix project config file for ASCF #18246
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
base: main
Are you sure you want to change the base?
Conversation
fix project config file for ASCF
Walkthrough在 AscfApp 的 setupTransaction 的 close 包装流程中,于调用 modifyTemplate 与 modifyWebpackConfig 之后,新增调用 generateProjectConfig('ascf.config.json', 'ascf.config.json'),以在 close 阶段生成 ascf.config.json。 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Developer
participant App as AscfApp
participant FS as FileSystem
Dev->>App: setupTransaction() 完成,进入 close 包装
App->>FS: modifyTemplate()
App->>FS: modifyWebpackConfig()
note right of App: 新增步骤
App->>FS: generateProjectConfig('ascf.config.json','ascf.config.json')
FS-->>App: 写入完成
App-->>Dev: close 完成
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type 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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/taro-platform-ascf/src/program.ts
(1 hunks)
🔇 Additional comments (2)
packages/taro-platform-ascf/src/program.ts (2)
40-40
: LGTM:新增 generateProjectConfig 调用确保 ascf.config.json 被编译进 dist调用位置放在 setupTransaction 的 close 阶段,顺序在 modifyTemplate/modifyWebpackConfig 之后,能解决 ASCF 配置文件未输出的问题,方向正确。
40-40
: 无需调整generateProjectConfig
签名或调用方式经核查:
基类
TaroPlatformBase
中定义protected generateProjectConfig(src: string, dist = 'project.config.json') { … }与文档注释(
@param dist
默认值为'project.config.json'
)一致,签名正确。各平台插件调用方式均显式传入了两个参数:
- swan:
this.generateProjectConfig(PROJECT_JSON, PROJECT_JSON)
- alipay:
this.generateProjectConfig('project.alipay.json', 'mini.project.json')
- ascf:
this.generateProjectConfig('ascf.config.json', 'ascf.config.json')
均符合基类的
(src: string, dist?: string)
约定。仅基类内部在处理 mini 平台时会省略第二个参数(使用默认值),属于基类自身逻辑,与各插件的调用无冲突。
结论:当前
ascf
平台对generateProjectConfig
的调用已与其他平台保持一致,无需修改或统一调用形式。
close () { | ||
this.modifyTemplate(pluginOptions) | ||
this.modifyWebpackConfig() | ||
this.generateProjectConfig('ascf.config.json', 'ascf.config.json') |
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
避免硬编码文件名,改用 this.projectConfigJson 以兼容自定义 projectConfigName
类中已定义 projectConfigJson = this.config.projectConfigName || 'ascf.config.json'
(见行 14)。此处硬编码 'ascf.config.json'
会忽略调用方通过 config.projectConfigName
的覆盖,破坏可配置性。建议改为使用类属性,或直接依赖默认参数。
建议修改如下:
- this.generateProjectConfig('ascf.config.json', 'ascf.config.json')
+ this.generateProjectConfig(this.projectConfigJson, this.projectConfigJson)
如果 generateProjectConfig
支持无参且默认取 this.projectConfigJson
,也可简化为:
- 直接调用
this.generateProjectConfig()
(仅作说明,不作为本 diff 的必改项)。
📝 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.
this.generateProjectConfig('ascf.config.json', 'ascf.config.json') | |
@@ packages/taro-platform-ascf/src/program.ts | |
this.generateProjectConfig(this.projectConfigJson, this.projectConfigJson) |
🤖 Prompt for AI Agents
In packages/taro-platform-ascf/src/program.ts around line 40 (class defines
projectConfigJson = this.config.projectConfigName || 'ascf.config.json' at line
14), the call this.generateProjectConfig('ascf.config.json', 'ascf.config.json')
hardcodes the filename and ignores caller-provided config.projectConfigName;
change the call to use the class property (this.projectConfigJson) instead of
the string literal, or if generateProjectConfig supports no arguments, call
this.generateProjectConfig() so the method uses the default projectConfigJson;
ensure both parameters (if still required) are replaced with
this.projectConfigJson to preserve configurability.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #18246 +/- ##
=======================================
Coverage 55.07% 55.07%
=======================================
Files 416 416
Lines 21561 21561
Branches 5301 5245 -56
=======================================
Hits 11875 11875
+ Misses 8035 8033 -2
- Partials 1651 1653 +2
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
这个 PR 做了什么? (简要描述所做更改)
fix project config file for ASCF, support compile ascf.config.json to dist
这个 PR 是什么类型? (至少选择一个)
这个 PR 涉及以下平台:
Summary by CodeRabbit