perf: improve regexp performance with non-capturing groups#13567
perf: improve regexp performance with non-capturing groups#13567edison1105 merged 4 commits intovuejs:mainfrom
Conversation
WalkthroughThis update replaces several capturing regex groups with non-capturing groups or equivalent string checks across multiple packages; behavior and public APIs remain unchanged. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as Code using regex
participant Regex as Regex engine / String API
Caller->>Regex: match pattern with capturing groups (old)
Note over Regex: engine captures subgroups
Caller->>Regex: match pattern with non-capturing groups or use string methods (new)
Note over Regex: matches without creating capture results
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
✨ 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 (
|
Size ReportBundles
Usages
|
@vue/compiler-core
@vue/compiler-dom
@vue/compiler-sfc
@vue/compiler-ssr
@vue/reactivity
@vue/runtime-core
@vue/runtime-dom
@vue/server-renderer
@vue/shared
vue
@vue/compat
commit: |
58faa41 to
1779b63
Compare
|
/ecosystem-ci run |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/compiler-core/src/utils.ts (2)
191-193: Also recognize generator function expressions (function*).Browser-side check currently misses
function*/async function*(Node path via Babel covers it). Low risk to include.Apply:
- /^\s*(?:async\s*)?(?:\([^)]*?\)|[\w$_]+)\s*(?::[^=]+)?=>|^\s*(?:async\s+)?function(?:\s+[\w$]+)?\s*\(/ + /^\s*(?:async\s*)?(?:\([^)]*?\)|[\w$_]+)\s*(?::[^=]+)?=>|^\s*(?:async\s+)?function\s*\*?\s*(?:\s+[\w$]+)?\s*\(/
191-193: Minor regex cleanup: dedupe the start anchor.Tiny simplification, same behavior.
- /^\s*(?:async\s*)?(?:\([^)]*?\)|[\w$_]+)\s*(?::[^=]+)?=>|^\s*(?:async\s+)?function\s*\*?\s*(?:\s+[\w$]+)?\s*\(/ + /^\s*(?:(?:async\s*)?(?:\([^)]*?\)|[\w$_]+)\s*(?::[^=]+)?=>|(?:async\s+)?function\s*\*?\s*(?:\s+[\w$]+)?\s*\()/
📜 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 (2)
packages/compiler-core/src/transforms/vIf.ts(1 hunks)packages/compiler-core/src/utils.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/compiler-core/src/transforms/vIf.ts
🔇 Additional comments (2)
packages/compiler-core/src/utils.ts (2)
191-193: Non-capturing groups change is good.Semantics preserved; avoids needless capture allocation in a hot regex.
191-193: Add a couple of TS edge-case tests.Ensure the browser regex returns true for tricky but valid cases:
(x: number): ((y: string) => void) => () => {}async function* foo() {}
And false for non-functions:async (x)(no arrow)(x: number): Foo(type-only)
|
📝 Ran ecosystem CI: Open
|
Use non-capturing groups or remove them as far as possible.
Summary by CodeRabbit