fix: 修复definePageConfig宏函数正则匹配注释代码问题。 #18195
Conversation
Walkthrough在 Changes
Sequence Diagram(s)sequenceDiagram
participant Reader as readSFCPageConfig
participant Cleaner as removeComments
participant Matcher as RegExp (definePageConfig)
Reader->>Cleaner: removeComments(sourceCode)
Cleaner-->>Reader: codeWithoutComments
Reader->>Matcher: match(codeWithoutComments)
Matcher-->>Reader: matchResult / null
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
✨ 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/taro-helper/src/utils.ts (2)
585-592: 正则去注释会误删字符串里的 “http://”“https://” 等内容，存在潜在回归风险当前实现用
/\/\/.*$/gm和/\/\*[\s\S]*?\*\//g直接删注释，会把字符串/模板字符串/正则字面量中的//或/* ... */一并清掉。典型场景：definePageConfig配置里若包含http://URL，则该行从//起被整行删除，可能导致括号或引号缺失，进而匹配不到definePageConfig或解析失败。这属于隐患型问题，建议加一道保护，至少不要删除协议前缀导致的//。建议的最小修正（保留协议型
://之前的//），在本段内可直接替换：export function removeComments(code: string): string { - // 移除单行注释 - code = code.replace(/\/\/.*$/gm, '') - // 移除多行注释 - code = code.replace(/\/\*[\s\S]*?\*\//g, '') - return code + // 移除单行注释（避免误删字符串中的协议前缀，如 http://、https://、ws://、ftp://、file://） + code = code.replace(/\/\/.*$/gm, (match: string, ...args: any[]) => { + const offset = args[args.length - 2] as number + const src = args[args.length - 1] as string + // 前一个字符为 ':' 大概率是协议场景，跳过删除 + if (offset > 0 && src[offset - 1] === ':') return match + return '' + }) + // 移除多行注释 + code = code.replace(/\/\*[\s\S]*?\*\//g, '') + return code }补充建议与验证点：
- 覆盖用例：含有
http:///https:///ws://等 URL 的definePageConfig能正常匹配与解析。- 可进一步（后续 PR）用状态机或引入 strip-comments 等库，实现“识别字符串/模板字符串/正则字面量后再去注释”的更稳妥版本。如需我来补充实现与测试，请告知。
599-603: 修复方向正确：先移除注释，再匹配 definePageConfig，能规避注释命中导致的误处理这一步正好解决了 PR 中描述的 false-positive 问题，思路合理、影响面可控。为提升健壮性，正则可以稍作放宽以容忍空白与换行，减少格式差异导致的漏匹配：
可以在本段微调正则，增强容错性（不改变原有语义）：
- const dpcReg = /definePageConfig\(\{[\w\W]+?\}\)/g + // 容忍空白与换行、保证单词边界，避免如“definePageConfig ({ ... })”之类的漏匹配 + const dpcReg = /\bdefinePageConfig\s*\(\s*\{[\s\S]*?\}\s*\)/g建议补充两类单测（或样例）验证：
- 注释中包含
definePageConfig({...})：应不匹配。- 实际代码中包含
definePageConfig({...})且对象里有包含http://的字符串：应能匹配并被后续 Babel 解析成功。
如需我来补充测试用例，我可以直接给出用 Jest 的最小可复现场景。
📜 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-helper/src/utils.ts(1 hunks)
|
优秀，优秀，优秀 |
| code = code.replace(/\/\*[\s\S]*?\*\//g, '') | ||
| return code | ||
| } | ||
|
|
There was a problem hiding this comment.
不清楚历史，代码解析有关的逻辑为啥不是AST处理的呢
There was a problem hiding this comment.
赞同用AST处理。同样不清楚历史业务，所以没有做过大的改动，仅按照原有的逻辑去修正了问题。
Codecov Report❌ Patch coverage is
❌ Your project check has failed because the head coverage (55.04%) is below the target coverage (75.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #18195 +/- ##
==========================================
- Coverage 55.06% 55.04% -0.02%
==========================================
Files 416 416
Lines 21563 21572 +9
Branches 5286 5274 -12
==========================================
+ Hits 11873 11874 +1
- Misses 8161 8173 +12
+ Partials 1529 1525 -4
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
3 participants
这个 PR 做了什么？ (简要描述所做更改)
注释的宏命令会产生影响，导致页面产生了分享设置无效的bug。定位到definePageConfig正则会匹配注释的代码，解决方案为：执行前过滤掉注释代码内容。
ps：可能还有别的宏函数也有类似问题，此处只修复了definePageConfig。
这个 PR 是什么类型？ (至少选择一个)
这个 PR 涉及以下平台：
Summary by CodeRabbit