feat: volar plugins #609
feat: volar plugins #609
Conversation
commit: |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #609 +/- ##
==========================================
- Coverage 62.75% 60.95% -1.80%
==========================================
Files 32 36 +4
Lines 3168 3373 +205
Branches 594 617 +23
==========================================
+ Hits 1988 2056 +68
- Misses 1175 1310 +135
- Partials 5 7 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
||
| // TODO: Do we want to apply this to EVERY .vue file or only to components that the user wrote themselves? | ||
|
|
||
| const relativeFilePath = ctx.compilerOptions.baseUrl |
There was a problem hiding this comment.
Will ctx.compilerOptions.baseUrl always equal the VueRouter plugin's root? If not, this might not always work.
| @@ -0,0 +1,7 @@ | |||
| <template> | |||
| <RouterView name="default" /> | |||
| <RouterView name="a" /> | |||
There was a problem hiding this comment.
You are typing these too? Pretty neat! It might be worth to adapt types in vue router too first?
There was a problem hiding this comment.
I think it was just an experiment, but not sure if I actually finished this. I'll check in a few days!
|
|
||
| const plugin: VueLanguagePlugin = (ctx) => { | ||
| const RE = { | ||
| USE_ROUTE: { |
There was a problem hiding this comment.
are regexes the recommended way? Isn't there an ast to traverse instead?
There was a problem hiding this comment.
If AST is truly possible, that would be better, but I noticed other Volar plugins usually use regexes too. Maybe @KazariEX could give us his two cents here :)
| return | ||
| } | ||
|
|
||
| // TODO: Do we want to apply this to EVERY .vue file or only to components that the user wrote themselves? |
There was a problem hiding this comment.
I think we only want to apply this to page components. The question might be how do we pass those paths from the config
There was a problem hiding this comment.
If a file isn't matched, because it's not in the generated RouteFileInfoMap, the route is generalized, I believe. But it needs extensive testing.
playground/typed-router.d.ts
Outdated
| */ | ||
| export type GetPossibleRouteNamesByFilePath<T extends string> = T extends keyof RouteFileInfoMap | ||
| ? RouteFileInfoMap[T]['routes'] | ||
| : keyof import('vue-router/auto-routes').RouteNamedMap |
There was a problem hiding this comment.
Why do we need the dynamic import?
There was a problem hiding this comment.
I think it had something to do with interface declaration merging. Maybe test if it works without the dynamic import when you extend the RouteNamedMap elsewhere in a project.
There was a problem hiding this comment.
Do you remember what test case was failing? I'm trying to see if it's okay to just do RouteNamedMap
|
|
||
| // TODO: Do we want to apply this to EVERY .vue file or only to components that the user wrote themselves? | ||
|
|
||
| const relativeFilePath = ctx.compilerOptions.baseUrl |
| ? relative(ctx.compilerOptions.baseUrl, fileName).replaceAll('\\', '/') | ||
| : fileName | ||
|
|
||
| const routeNameGetter = `import('vue-router/auto-routes').GetPossibleRouteNamesByFilePath<'${relativeFilePath}'>` |
There was a problem hiding this comment.
now we should be able to just pass the name of the route associated with the file
WalkthroughThis update introduces new Volar language plugins for handling Vue SFC route blocks and typed router support, updates TypeScript declaration generation to include route file info mapping, and adds new runtime dependencies. Several playground Vue components are adjusted for route usage, and new components for named views are added. The Volar plugin implementation is refactored from CommonJS to TypeScript modules. Changes
Possibly related PRs
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 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 (2)
✨ Finishing Touches
🧪 Generate Unit Tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/volar/utils/augment-vls-ctx.ts (1)
26-26: Consider documenting array mutation behavior.The function mutates the input array directly, which may be unexpected. Consider adding JSDoc to clarify this behavior.
+/** + * Augments VLS context by injecting code before the first semicolon after __VLS_ctx declaration. + * @param content - Array of Code elements (will be mutated) + * @param getCodes - Function that returns code to inject + */ export function augmentVlsCtx(content: Code[], getCodes: () => ` & ${string}`) {playground/src/pages/named-views/parent.vue (1)
5-6: Address the TODO comment for the "c" named view.The @vue-expect-error with TODO indicates incomplete implementation. Consider either implementing the corresponding component or removing this RouterView if it's not needed.
tsdown-runtime.config.ts (1)
24-24: Address placement uncertainty.The TODO comment suggests uncertainty about config placement. The current location is reasonable for now, but consider a separate config file if Volar builds become more complex.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (26)
package.json(2 hunks)playground/src/pages/@[profileId].vue(1 hunks)playground/src/pages/[...path]+.vue(2 hunks)playground/src/pages/[...path].vue(1 hunks)playground/src/pages/[name].vue(3 hunks)playground/src/pages/custom-name-and-path.vue(1 hunks)playground/src/pages/deep/nesting/works/custom-name-and-path.vue(0 hunks)playground/src/pages/named-views/parent.vue(1 hunks)playground/src/pages/named-views/parent/index.vue(1 hunks)playground/src/pages/named-views/parent/index@a.vue(1 hunks)playground/src/pages/named-views/parent/index@b.vue(1 hunks)playground/src/pages/partial-[name].vue(1 hunks)playground/src/pages/test-[a-id].vue(1 hunks)playground/src/pages/users/[id].vue(1 hunks)playground/src/pages/users/colada-loader.[id].vue(1 hunks)playground/tsconfig.json(1 hunks)playground/typed-router.d.ts(2 hunks)src/codegen/generateDTS.ts(2 hunks)src/codegen/generateRouteFileInfoMap.spec.ts(1 hunks)src/codegen/generateRouteFileInfoMap.ts(1 hunks)src/core/context.ts(2 hunks)src/volar/entries/sfc-route-blocks.ts(1 hunks)src/volar/entries/sfc-typed-router.ts(1 hunks)src/volar/utils/augment-vls-ctx.ts(1 hunks)tsdown-runtime.config.ts(1 hunks)volar/index.cjs(0 hunks)
💤 Files with no reviewable changes (2)
- playground/src/pages/deep/nesting/works/custom-name-and-path.vue
- volar/index.cjs
🧰 Additional context used
🧬 Code Graph Analysis (5)
src/codegen/generateRouteFileInfoMap.spec.ts (3)
src/options.ts (1)
resolveOptions(295-352)src/core/tree.ts (1)
PrefixTree(277-311)src/codegen/generateRouteFileInfoMap.ts (1)
generateRouteFileInfoMap(8-18)
src/core/context.ts (1)
src/codegen/generateRouteFileInfoMap.ts (1)
generateRouteFileInfoMap(8-18)
tsdown-runtime.config.ts (1)
tsdown.config.ts (1)
commonOptions(3-13)
src/codegen/generateRouteFileInfoMap.ts (2)
src/core/tree.ts (1)
PrefixTree(277-311)src/core/extendRoutes.ts (1)
children(190-194)
src/volar/entries/sfc-typed-router.ts (1)
src/volar/utils/augment-vls-ctx.ts (1)
augmentVlsCtx(3-27)
🔇 Additional comments (43)
src/volar/utils/augment-vls-ctx.ts (1)
3-27: 🛠️ Refactor suggestionHandle edge case where start pattern is found but end pattern isn't.
The function returns early if
to === -1but doesn't resetfromwhen the end pattern isn't found after finding the start pattern.if (to === -1) { + // Reset from if end pattern not found + from = -1 return }Likely an incorrect or invalid review comment.
playground/src/pages/test-[a-id].vue (1)
3-3: Good simplification for improved type inference.Removing the explicit route path allows the new Volar plugins to provide better type safety through automatic inference.
playground/src/pages/named-views/parent/index@a.vue (1)
1-3: Simple and appropriate for named views demo.Clean implementation for demonstrating named view functionality.
playground/src/pages/users/[id].vue (1)
44-44: Consistent with type inference improvements.Removing explicit route path aligns with the new Volar plugin approach for automatic type inference.
playground/src/pages/@[profileId].vue (1)
3-3: Change aligns with new Volar plugin infrastructure.Removing the explicit route path argument is consistent with the enhanced type inference provided by the new Volar plugins.
playground/src/pages/users/colada-loader.[id].vue (1)
12-12: Consistent with new typing approach.Removing the explicit route path follows the pattern for leveraging enhanced type inference.
playground/src/pages/named-views/parent/index.vue (1)
1-3: Simple and appropriate for named views demo.Clean implementation that effectively demonstrates the default named view.
playground/tsconfig.json (1)
38-39: Properly configures new Volar plugins.The plugin configuration correctly replaces the legacy plugin with the new modular approach.
playground/src/pages/named-views/parent/index@b.vue (1)
1-3: LGTM!Simple, valid Vue component for named view "b".
playground/src/pages/partial-[name].vue (1)
2-2: API improvement aligns with pattern.Simplified useRoute() call is consistent with broader changes across route components.
playground/src/pages/[...path].vue (2)
4-4: Template simplified correctly.Removing conditional check and directly displaying route params is cleaner.
8-8: Good addition for Volar plugin support.Adding
lang="json"enables better tooling support for route blocks.playground/src/pages/[...path]+.vue (2)
4-4: Consistent API simplification.useRoute() change follows the established pattern across route components.
14-14: Enables better tooling support.Adding
lang="json"to route block aligns with Volar plugin enhancements.playground/src/pages/custom-name-and-path.vue (1)
13-15: Good demonstration of enhanced type safety.The type assertion correctly validates the route name against the expected literal type, showcasing the improved typing capabilities.
playground/src/pages/[name].vue (3)
52-52: Good addition of RouteNamedMap import for enhanced typing.
94-95: LGTM on standardized useRoute calls.The removal of explicit route path arguments and use of generic type parameters aligns with the broader refactoring for improved type inference.
137-137: Template expression correctly simplified.Removing the conditional check is appropriate since the route parameter is guaranteed to be available.
src/core/context.ts (2)
6-6: Import added correctly for route file info map generation.
245-247: Proper integration of route file info map generation.The function call correctly passes the routeTree and root options to generate enhanced type information for route files.
src/codegen/generateRouteFileInfoMap.spec.ts (1)
1-87: LGTM! Comprehensive test coverage.The test suite properly covers the main scenarios for route file info map generation. The snapshots correctly reflect that parent routes with children should have
views: 'default'while leaf routes haveviews: never.package.json (2)
80-85: Missing ESM exports for consistency.The new Volar plugin exports only provide CommonJS (
require) paths but lack ESM (import) paths, unlike other exports in this file. Is this intentional due to Volar's runtime requirements?
142-152: LGTM! Dependencies support new Volar functionality.The added dependencies (
@babel/types,@vue/language-core,muggle-string) are appropriate for the new Volar plugin implementation.tsdown-runtime.config.ts (1)
25-39: LGTM! Build configuration is appropriate.The Volar build config correctly uses CJS format to match the package.json exports and properly lists external dependencies.
src/codegen/generateDTS.ts (3)
3-12: LGTM! Good refactoring to eliminate duplication.The
normalizeLineshelper function properly extracts the common formatting logic for reuse.
17-22: LGTM! Clean parameter addition.The
routeFileInfoMapparameter is properly integrated into the function signature.
44-56: LGTM! Proper integration of route file info mapping.The route file info map is correctly included in the generated module, and the
GetPossibleRouteNamesByFilePathtype provides a useful conditional lookup with appropriate fallback.src/volar/entries/sfc-route-blocks.ts (4)
4-18: Plugin structure and initialization look good.The plugin correctly implements the VueLanguagePlugin interface and properly handles route block enumeration.
29-46: Robust embedded code resolution with proper bounds checking.Good defensive programming with the block existence check and proper content mapping to embedded code.
48-60: JSON schema injection implementation.The schema injection logic correctly identifies JSON objects and adds the schema reference. The URL points to the correct repository location.
21-25:❓ Verification inconclusive
Verify language mapping for route blocks.
The fallback from 'txt' to 'json' addresses the TODO limitation but may need verification that this mapping is consistent with Vue language tools behavior.
🏁 Script executed:
#!/bin/bash # Description: Check how Vue language tools handle custom block language detection # Expected: Find documentation or examples of custom block language handling rg -A 5 -B 5 "custom.*block.*lang" --type ts --type jsLength of output: 56
🏁 Script executed:
#!/bin/bash sed -n '1,200p' src/volar/entries/sfc-route-blocks.tsLength of output: 2132
Confirm fallback from ‘txt’ to ‘json’ aligns with Vue’s behavior.
I couldn’t find any explicit mapping for custom‐block languages in the Vue language tools. Please verify that a
<route>block withoutlang="json"is indeed treated as JSON by the Vue tooling. You can test this by authoring a<route>block without anylangattribute and checking how it’s parsed in your editor or language server.src/volar/entries/sfc-typed-router.ts (4)
16-32: Regex patterns for code transformation.The regex patterns correctly target
useRoute()calls and$routeusage. Using named capture groups and lookbehind/lookahead assertions provides precise targeting.
43-49: File path resolution and type helper usage.The relative path calculation and type helper construction look correct. The import statement properly references the generated types.
51-68: TypeScript and JavaScript code transformations.Both transformation approaches (generic injection for TS, type casting for JS) are implemented correctly and handle the different language contexts appropriately.
74-91: VLS context augmentation for template typing.The
$routetyping augmentation correctly uses the utility function and provides proper type overrides for template usage.playground/typed-router.d.ts (3)
58-59: New named views routes properly defined.The route entries for named views are correctly structured with proper parameter and query types.
83-320: Comprehensive file-to-route mapping interface.The RouteFileInfoMap provides thorough mapping of component files to their route names and views. The structure correctly handles both simple routes and complex nested/named view scenarios.
325-327: Type helper with proper fallback handling.The conditional type correctly resolves route names by file path and falls back to all route names when the file isn't found in the map.
src/codegen/generateRouteFileInfoMap.ts (5)
8-18: Function signature and root validation.Good defensive programming with proper root node validation and clear error messaging.
24-41: Route names and named views collection logic.The logic correctly collects route names recursively and extracts unique named views from component keys. The Set usage prevents duplicates.
46-58: Component mapping and recursive traversal.The code properly maps components to file info entries and recursively processes child nodes. The flat array join ensures proper nesting structure.
67-80: File info entry generation with proper path handling.The relative path calculation and template generation correctly format the interface entries. Cross-platform path handling with
replaceAll('\\', '/')is appropriate.
85-90: Recursive route name collection.The recursive function correctly flattens the tree structure to collect all route names from a node and its descendants.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
playground/typed-router.d.ts (1)
325-327: Address previous feedback on dynamic import usage.The implementation still uses dynamic import despite previous suggestions to avoid it and make the type internal.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (26)
package.json(2 hunks)playground/src/pages/@[profileId].vue(1 hunks)playground/src/pages/[...path]+.vue(2 hunks)playground/src/pages/[...path].vue(1 hunks)playground/src/pages/[name].vue(3 hunks)playground/src/pages/custom-name-and-path.vue(1 hunks)playground/src/pages/deep/nesting/works/custom-name-and-path.vue(0 hunks)playground/src/pages/named-views/parent.vue(1 hunks)playground/src/pages/named-views/parent/index.vue(1 hunks)playground/src/pages/named-views/parent/index@a.vue(1 hunks)playground/src/pages/named-views/parent/index@b.vue(1 hunks)playground/src/pages/partial-[name].vue(1 hunks)playground/src/pages/test-[a-id].vue(1 hunks)playground/src/pages/users/[id].vue(1 hunks)playground/src/pages/users/colada-loader.[id].vue(1 hunks)playground/tsconfig.json(1 hunks)playground/typed-router.d.ts(2 hunks)src/codegen/generateDTS.ts(2 hunks)src/codegen/generateRouteFileInfoMap.spec.ts(1 hunks)src/codegen/generateRouteFileInfoMap.ts(1 hunks)src/core/context.ts(2 hunks)src/volar/entries/sfc-route-blocks.ts(1 hunks)src/volar/entries/sfc-typed-router.ts(1 hunks)src/volar/utils/augment-vls-ctx.ts(1 hunks)tsdown-runtime.config.ts(1 hunks)volar/index.cjs(0 hunks)
💤 Files with no reviewable changes (2)
- playground/src/pages/deep/nesting/works/custom-name-and-path.vue
- volar/index.cjs
✅ Files skipped from review due to trivial changes (3)
- playground/src/pages/named-views/parent/index@a.vue
- src/codegen/generateRouteFileInfoMap.spec.ts
- src/volar/entries/sfc-route-blocks.ts
🚧 Files skipped from review as they are similar to previous changes (20)
- playground/src/pages/users/[id].vue
- playground/src/pages/test-[a-id].vue
- playground/src/pages/users/colada-loader.[id].vue
- playground/src/pages/named-views/parent/index@b.vue
- playground/src/pages/partial-[name].vue
- playground/src/pages/named-views/parent/index.vue
- playground/src/pages/[...path].vue
- playground/src/pages/@[profileId].vue
- playground/src/pages/[name].vue
- playground/src/pages/[...path]+.vue
- playground/tsconfig.json
- playground/src/pages/custom-name-and-path.vue
- playground/src/pages/named-views/parent.vue
- src/core/context.ts
- tsdown-runtime.config.ts
- package.json
- src/volar/utils/augment-vls-ctx.ts
- src/codegen/generateRouteFileInfoMap.ts
- src/codegen/generateDTS.ts
- src/volar/entries/sfc-typed-router.ts
🔇 Additional comments (1)
playground/typed-router.d.ts (1)
58-59: LGTM: Named view routes added correctly.The new route entries follow the existing pattern and are properly typed.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/codegen/generateRouteFileInfoMap.ts (2)
73-73: Use proper path normalization.
replaceAll('\\', '/')may not handle all edge cases. Usepath.posix.normalize()or similar.- relative(options.root, file).replaceAll('\\', '/') + relative(options.root, file).split(path.sep).join('/')
75-75: Address TODO comment for views implementation.The TODO suggests the views implementation needs verification.
Do you want me to help investigate the views implementation or create an issue to track this?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (26)
package.json(2 hunks)playground/src/pages/@[profileId].vue(1 hunks)playground/src/pages/[...path]+.vue(2 hunks)playground/src/pages/[...path].vue(1 hunks)playground/src/pages/[name].vue(3 hunks)playground/src/pages/custom-name-and-path.vue(1 hunks)playground/src/pages/deep/nesting/works/custom-name-and-path.vue(0 hunks)playground/src/pages/named-views/parent.vue(1 hunks)playground/src/pages/named-views/parent/index.vue(1 hunks)playground/src/pages/named-views/parent/index@a.vue(1 hunks)playground/src/pages/named-views/parent/index@b.vue(1 hunks)playground/src/pages/partial-[name].vue(1 hunks)playground/src/pages/test-[a-id].vue(1 hunks)playground/src/pages/users/[id].vue(1 hunks)playground/src/pages/users/colada-loader.[id].vue(1 hunks)playground/tsconfig.json(1 hunks)playground/typed-router.d.ts(2 hunks)src/codegen/generateDTS.ts(2 hunks)src/codegen/generateRouteFileInfoMap.spec.ts(1 hunks)src/codegen/generateRouteFileInfoMap.ts(1 hunks)src/core/context.ts(2 hunks)src/volar/entries/sfc-route-blocks.ts(1 hunks)src/volar/entries/sfc-typed-router.ts(1 hunks)src/volar/utils/augment-vls-ctx.ts(1 hunks)tsdown-runtime.config.ts(1 hunks)volar/index.cjs(0 hunks)
💤 Files with no reviewable changes (2)
- playground/src/pages/deep/nesting/works/custom-name-and-path.vue
- volar/index.cjs
✅ Files skipped from review due to trivial changes (1)
- src/codegen/generateRouteFileInfoMap.spec.ts
🚧 Files skipped from review as they are similar to previous changes (22)
- playground/src/pages/@[profileId].vue
- playground/src/pages/users/[id].vue
- playground/src/pages/users/colada-loader.[id].vue
- playground/src/pages/test-[a-id].vue
- playground/src/pages/[...path]+.vue
- playground/src/pages/named-views/parent/index@b.vue
- playground/src/pages/named-views/parent/index@a.vue
- playground/src/pages/named-views/parent/index.vue
- playground/src/pages/[...path].vue
- playground/src/pages/partial-[name].vue
- playground/tsconfig.json
- src/core/context.ts
- playground/src/pages/[name].vue
- src/volar/entries/sfc-route-blocks.ts
- tsdown-runtime.config.ts
- playground/src/pages/custom-name-and-path.vue
- src/volar/utils/augment-vls-ctx.ts
- src/codegen/generateDTS.ts
- src/volar/entries/sfc-typed-router.ts
- package.json
- playground/src/pages/named-views/parent.vue
- playground/typed-router.d.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/codegen/generateRouteFileInfoMap.ts (2)
src/core/tree.ts (1)
PrefixTree(277-311)src/core/extendRoutes.ts (1)
children(190-194)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/codegen/generateRouteFileInfoMap.ts (1)
94-94: Handle empty route names union.If
routesNamesis empty, this generates an invalid TypeScript union type.- routes: ${routesNames.join(' | ')} + routes: ${routesNames.length > 0 ? routesNames.join(' | ') : 'never'}
🧹 Nitpick comments (1)
src/codegen/generateRouteFileInfoMap.ts (1)
16-16: Address FIXME/TODO comments before merge.Multiple areas marked for improvement should be resolved.
Would you like me to help implement the fixes mentioned in these comments?
Also applies to: 31-31, 51-51, 86-86, 90-91
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
playground/typed-router.d.ts(2 hunks)src/codegen/generateRouteFileInfoMap.spec.ts(1 hunks)src/codegen/generateRouteFileInfoMap.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/codegen/generateRouteFileInfoMap.spec.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- playground/typed-router.d.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/codegen/generateRouteFileInfoMap.ts (2)
src/core/tree.ts (1)
PrefixTree(277-311)src/core/extendRoutes.ts (1)
children(190-194)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Socket Security: Pull Request Alerts
🔇 Additional comments (2)
src/codegen/generateRouteFileInfoMap.ts (2)
4-25: LGTM - Main function structure is solid.The validation and template generation logic is correct.
59-69: 🛠️ Refactor suggestionLogic inconsistency with empty route names.
When
routeNames.length === 0, the function returns an empty array but still processes components. This could generate invalid entries.Consider this fix:
const currentRouteInfo = routeNames.length === 0 - ? [] + ? [] : Array.from(node.value.components.values()).map((file) => generateRouteFileInfoEntry( file, routeNames, childrenNamedViewsUnion, rootDir ) )Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/codegen/generateRouteFileInfoMap.ts (1)
45-45: Handle empty routes array to prevent invalid TypeScript.If the routes array is empty, this generates an empty string instead of a valid TypeScript union type.
- routes: ${routes.map((name) => `'${name}'`).join(' | ')} + routes: ${routes.length > 0 ? routes.map((name) => `'${name}'`).join(' | ') : 'never'}
🧹 Nitpick comments (2)
src/codegen/generateRouteFileInfoMap.ts (2)
88-88: Use path normalization library instead of string replacement.Manual path separator replacement may not handle all edge cases across platforms.
- key: relative(rootDir, file).replaceAll('\\', '/'), + key: relative(rootDir, file).replace(/\\/g, '/'),
68-68: Simplify children collection logic.The conditional can be simplified since
getSortedChildrenDeep()should handle empty children gracefully.- const children = node.children.size > 0 ? node.getSortedChildrenDeep() : null + const children = node.getSortedChildrenDeep() + const hasChildren = children.length > 0
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
playground/typed-router.d.ts(2 hunks)src/codegen/generateRouteFileInfoMap.spec.ts(1 hunks)src/codegen/generateRouteFileInfoMap.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/codegen/generateRouteFileInfoMap.spec.ts
- playground/typed-router.d.ts
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Socket Security: Pull Request Alerts
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
package.json(2 hunks)playground/src/pages/articles.vue(1 hunks)playground/typed-router.d.ts(2 hunks)src/codegen/generateDTS.ts(2 hunks)src/codegen/generateRouteFileInfoMap.spec.ts(1 hunks)src/codegen/generateRouteFileInfoMap.ts(1 hunks)src/volar/entries/sfc-typed-router.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- src/codegen/generateRouteFileInfoMap.spec.ts
- playground/src/pages/articles.vue
- package.json
- src/codegen/generateRouteFileInfoMap.ts
- playground/typed-router.d.ts
- src/volar/entries/sfc-typed-router.ts
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Socket Security: Pull Request Alerts
🔇 Additional comments (1)
src/codegen/generateDTS.ts (1)
21-21:✅ Verification successful
Verify backward compatibility of function signature change.
Adding the required
routeFileInfoMapparameter is a breaking change. Ensure all callers have been updated accordingly.Also applies to: 26-26
🏁 Script executed:
#!/bin/bash # Description: Verify all callers of generateDTS have been updated with the new parameter rg -A 10 "generateDTS\(" --type tsLength of output: 1751
generateDTS signature update is safe
The only invocation of the codegen function lives insrc/core/context.tsand already suppliesrouteFileInfoMapvia the_generateDTSwrapper. No other callers were found.
src/volar/utils/augment-vls-ctx.ts
Outdated
| return | ||
| } | ||
|
|
||
| content.splice(to, 0, ...getCodes()) |
There was a problem hiding this comment.
This seems to work with just getCodes() (no spread). Is there a reason to spread the code character per character?
There was a problem hiding this comment.
I copied this bit from something @KazariEX created. But Array.prototype.splice is designed like this AFAIK: splice(start, deleteCount, item1, item2, /* …, */ itemN)
There was a problem hiding this comment.
getCodes should be () => Code[] instead of () => string.
There was a problem hiding this comment.
This part is still not clear to me. I struggle a lot to find comprehensible docs:
- https://deepwiki.com/vuejs/language-tools/3.1-vue-language-core with the AI search doesn't really help. If anything it confuses me more because it does suggest I can just push a string into the array of Code[]
- But muggle string suggest I don't need this for v2 plugins (this one is a v2 plugin)
I think I will merge this without the ... (it works) and add a TODO for later.
…olar plugin sfc-route-blocks
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
package.json(3 hunks)playground/src/components/TestSetup.vue(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: test (lts/*, windows-latest)
- GitHub Check: test (18.x, windows-latest)
🔇 Additional comments (1)
playground/src/components/TestSetup.vue (1)
4-6: Good type safety implementation.The
useRoute()call and type assertion usingsatisfiescorrectly ensure type safety for the route name.
| @@ -1,5 +1,9 @@ | |||
| <script setup lang="ts"> | |||
| import { onBeforeRouteLeave } from 'vue-router' | |||
| import { onBeforeRouteLeave, type RouteMap } from 'vue-router' | |||
There was a problem hiding this comment.
Missing import for useRoute.
The RouteMap type import is correct, but useRoute is used on line 4 without being imported.
-import { onBeforeRouteLeave, type RouteMap } from 'vue-router'
+import { onBeforeRouteLeave, useRoute, type RouteMap } from 'vue-router'📝 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.
| import { onBeforeRouteLeave, type RouteMap } from 'vue-router' | |
| -import { onBeforeRouteLeave, type RouteMap } from 'vue-router' | |
| +import { onBeforeRouteLeave, useRoute, type RouteMap } from 'vue-router' |
🤖 Prompt for AI Agents
In playground/src/components/TestSetup.vue at line 2, the import statement
includes onBeforeRouteLeave and RouteMap from 'vue-router' but is missing the
import for useRoute, which is used on line 4. Add useRoute to the import
statement alongside onBeforeRouteLeave and RouteMap to properly import it from
'vue-router'.
|
@Anoesj unfortunately one of my routes don't want to automatically infer the route type: relevant typed-router.d.ts: Also I think that That's because you can't refer to |
|
@darkbasic Thanks for the feedback! Would it be possible to create a repro of the first issue (public repo, because testing this in Stackblitz doesn't fully work as far as I'm aware of)? The second issue seems like a separate request. Could you open a new issue for that? |
|
@Anoesj here it is: https://github.com/darkbasic/unplugin-vue-router-609 |
<route>block IntelliSense intosrc, added it to the building process and added it to exportsvueCompilerOptions.plugins)useRouteand$routetypingsvueCompilerOptions.plugins)Future
<RouterView>slots regarding named viewsSummary by CodeRabbit
<route>blocks and providing typeduseRoute()calls.@vue/language-core,muggle-string, andvue-tsc.