Skip to content
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions packages/router/src/unplugin/codegen/generateRouteRecords.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,17 @@ describe('generateRouteRecord', () => {
expect(generateRouteRecordSimple(tree)).toContain("path: '/nested'")
})

it('skips nested lone _parent files', () => {
const tree = new PrefixTree(DEFAULT_OPTIONS)
tree.insert('users/index', 'users/index.vue')
tree.insert('users/settings/_parent', 'users/settings/_parent.vue')

const routes = generateRouteRecordSimple(tree)

expect(routes).toContain("path: '/users'")
expect(routes).not.toContain("path: '/users/settings'")
})
Comment on lines +51 to +60
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While this test verifies that route generation correctly skips nested lone _parent files, it doesn't test the dynamic deletion scenario that the bug fix addresses. Consider adding a test in tree.spec.ts that inserts a parent node with children, then deletes the children (using node.delete()), and verifies that the parent node is also removed. This would directly test the recursive cleanup logic added to the delete() method.

Copilot uses AI. Check for mistakes.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment is relevant

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment is relevant

Thank for you review, done!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry, my comment wasn't clear: you don't necessarily need to remove your test, it's great to have an e2e test like that but it still need the deletion part to get closer to the scenario you were describing with a deletion. IIRC there are EditableTree tests, which should feel closer to your actual scenario


it('works with some paths at root', () => {
const tree = new PrefixTree(DEFAULT_OPTIONS)
tree.insert('a', 'a.vue')
Expand Down
5 changes: 5 additions & 0 deletions packages/router/src/unplugin/core/tree.ts
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,11 @@ export class TreeNode {
throw new Error('Cannot delete the root node.')
}
this.parent.children.delete(this.value.rawSegment)
// delete lone parent nodes - they only provide layout wrapping for children
// so without children they don't make sense to be included in the route records
if (!this.parent.isMatchable() && this.parent.children.size === 0) {
this.parent.delete()
}
// clear link to parent
this.parent = undefined
}
Expand Down
Loading