feat(unplugin): allow null filePath for editable tree routes - #2748
feat(unplugin): allow null filePath for editable tree routes#2748Anoesj wants to merge 2 commits into
Conversation
Port posva/unplugin-vue-router#607. `EditableTreeNode.insert()` and `TreeNode.insertParsedPath()` now accept `filePath: string | null`, and a default component is only registered when `filePath != null`. This allows creating pass-through routes via the editable tree. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
✅ Deploy Preview for vue-router canceled.
|
commit: |
📝 WalkthroughWalkthroughRoute tree insertion now accepts ChangesNullable route file paths
Estimated code review effort: 2 (Simple) | ~5 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/router/src/unplugin/core/extendRoutes.spec.ts (1)
38-45: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert that the nullable route is non-component.
This test currently verifies only insertion and path creation; it would still pass if
nullwere incorrectly registered as a component. Add an assertion for the intended pass-through behavior.Proposed test addition
editable.insert('foo', null) expect(editable.children).toHaveLength(1) expect(editable.children[0]?.path).toBe('/foo') + expect(editable.children[0]?.isPassThrough).toBe(true)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/router/src/unplugin/core/extendRoutes.spec.ts` around lines 38 - 45, Extend the test for EditableTreeNode.insert with a null filePath to assert that the created route is non-component, using the route/component property exposed by editable.children[0]. Preserve the existing child-count and path assertions while verifying null passes through without component registration.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/router/src/unplugin/core/extendRoutes.spec.ts`:
- Around line 38-45: Extend the test for EditableTreeNode.insert with a null
filePath to assert that the created route is non-component, using the
route/component property exposed by editable.children[0]. Preserve the existing
child-count and path assertions while verifying null passes through without
component registration.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 532d7142-84de-4fa7-9d07-33b8cb7e26be
📒 Files selected for processing (3)
packages/router/src/unplugin/core/extendRoutes.spec.tspackages/router/src/unplugin/core/extendRoutes.tspackages/router/src/unplugin/core/tree.ts
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2748 +/- ##
==========================================
+ Coverage 86.91% 86.93% +0.01%
==========================================
Files 93 93
Lines 10715 10715
Branches 2474 2476 +2
==========================================
+ Hits 9313 9315 +2
+ Misses 1396 1394 -2
Partials 6 6 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Ported from posva/unplugin-vue-router#607
It seems that, when we only allow
filePathto benullon a route added in an editable tree, that shouldn't cause any problems.In Nuxt, we don't use file scanning and add routes by modifying the
EditableTreeNodeof the root page in thebeforeWriteFilescallback. It's possible for aNuxtPagenot to have a correspondingfile: modules can push routes through thepages:extendhook without one (virtual, grouping, or redirect routes), and Nuxt's own type fixtures do this. So where we runparent.insert(path, page.file),page.filecan be missing, and we currently use a@ts-expect-errorbefore that line to suppress TS errors.Nuxt follow-up (not part of this PR)
page.file ?? nullinaddPage, then remove the@ts-expect-error.Summary by CodeRabbit
New Features
Bug Fixes
Tests