-
-
Notifications
You must be signed in to change notification settings - Fork 105
fix: avoid empty param names breaking dts syntax #706
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
WalkthroughAdjusts generateRouteParams to use an empty string when paramName is falsy, preventing unintended identifiers; other mapping logic remains unchanged. No public APIs or types modified. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 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 (
|
commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #706 +/- ##
=======================================
Coverage 60.90% 60.90%
=======================================
Files 36 36
Lines 3379 3379
Branches 618 618
=======================================
Hits 2058 2058
Misses 1314 1314
Partials 7 7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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: 0
🧹 Nitpick comments (1)
src/codegen/generateRouteParams.ts (1)
10-10
: Consider centralizing key quoting to handle non-identifiers and escaping.If a param name includes characters not valid in TS identifiers (e.g., hyphens) or reserved words, quoting via a helper avoids syntax issues and keeps behavior consistent for all keys, not only empty ones.
Apply this diff locally to route through a formatter:
- `${param.paramName || `''`}${param.optional ? '?' : ''}: ` + + `${formatParamKey(param.paramName)}${param.optional ? '?' : ''}: ` +Add this helper near the file top:
function formatParamKey(name?: string): string { if (!name) return "''" // empty-string key const isIdent = /^[$A-Z_][0-9A-Z_$]*$/i.test(name) return isIdent ? name : `'${name.replace(/\\/g, '\\\\').replace(/'/g, "\\'")}'` }
📜 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)
src/codegen/generateRouteParams.ts
(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-15T16:11:02.627Z
Learnt from: posva
PR: posva/unplugin-vue-router#700
File: src/codegen/generateRouteResolver.ts:0-0
Timestamp: 2025-08-15T16:11:02.627Z
Learning: In src/codegen/generateRouteResolver.ts, the user wants comment alignment preserved in the generated resolver code, even when fixing potential runtime errors with String.repeat().
Applied to files:
src/codegen/generateRouteParams.ts
🔇 Additional comments (2)
src/codegen/generateRouteParams.ts (2)
10-10
: Fix: empty param names now emit a valid TS key ('').This prevents
?:
/:
from breaking the generated d.ts when the name is empty. Good targeted patch.
10-10
: No changes needed:paramName
is always a string
- The
TreeRouteParam
interface definessoexport interface TreeRouteParam { paramName: string; … }paramName
can’t be a numeric0
or any other non‐string value (treeNodeValue.ts:210).- All assignments to
currentTreeRouteParam.paramName
come from string buffers built from path segments, never numeric literals.The
|| ''
fallback only handles the empty‐string case and does not mask any valid numeric names.
Summary by CodeRabbit