Skip to content

fix(unplugin): Avoid generating empty routes #2642

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.

Sign up for GitHub

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

Open
FrontEndDog wants to merge 9 commits into
base: main
Choose a base branch
from
+46 −2
Open

fix(unplugin): Avoid generating empty routes #2642

Changes from 2 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
646bbf5
fix(unplugin): Avoid generating empty routes
FrontEndDog Feb 26, 2026
a07b7fe
fix(generateRouteRecords): remove lone children nodes from route records
FrontEndDog Feb 27, 2026
9dcdf60
fix(generateRouteRecords): skip nested lone _parent files in route ge…
FrontEndDog Feb 27, 2026
0f610c3
fix(generateRouteRecords): correct comment to reflect deletion of lon…
FrontEndDog Feb 27, 2026
e41658f
fix(generateRouteRecords): skip deletion of lone parent nodes in rout…
FrontEndDog Feb 27, 2026
c4e2148
fix(TreeNode): prevent deletion of root nodes without children in rou…
FrontEndDog Feb 27, 2026
4f9708a
fix(tree): add test for removing parent when last child of non-matcha…
FrontEndDog Feb 27, 2026
665ce2e
fix(tree): remove unnecessary insertion of parent node in test case
FrontEndDog Feb 27, 2026
2505de9
fix(tree): add test for removing parent when last child of a non-matc…
FrontEndDog Feb 27, 2026
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
14 changes: 8 additions & 6 deletions packages/router/src/unplugin/codegen/generateRouteRecords.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,14 @@ export function generateRouteRecords(
importsMap: ImportsMap,
indent = 0
): string {
// delete lone children nodes - they only provide layout wrapping for children
// so without children they don't make sense to be included in the route records
node.children.forEach(child => {
if (!child.isMatchable() && child.children.size === 0) {
child.delete()
}
})
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 27, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's look at the full context of the file and the changes
head -100 packages/router/src/unplugin/codegen/generateRouteRecords.ts | cat -n

Repository: vuejs/router

Length of output: 3625

🏁 Script executed:

# Find the TreeNode class to understand the delete() method
find . -type f -name "*.ts" -o -name "*.tsx" | head -20

Repository: vuejs/router

Length of output: 1011

🏁 Script executed:

# Search for TreeNode class definition and delete() method
rg "class TreeNode" -A 20

Repository: vuejs/router

Length of output: 5609

🏁 Script executed:

# Search for the delete() method specifically
rg "delete\(\)" -B 3 -A 8 packages/router/src/

Repository: vuejs/router

Length of output: 5112

🏁 Script executed:

# Let's examine the full delete() method and surrounding code
sed -n '80,95p' packages/router/src/unplugin/core/tree.ts | cat -n

Repository: vuejs/router

Length of output: 718

🏁 Script executed:

# Check if there are any tests or comments about this behavior
rg "forEach.*delete" packages/router/src/ -B 3 -A 3

Repository: vuejs/router

Length of output: 38

🏁 Script executed:

# Look for any post-processing or cleanup passes that might handle cascade pruning
rg "cascad|cleanup|post" packages/router/src/unplugin/ -i

Repository: vuejs/router

Length of output: 2085

🏁 Script executed:

# Let's verify the full delete() method context and see the Map behavior
sed -n '60,100p' packages/router/src/unplugin/core/tree.ts | cat -n

Repository: vuejs/router

Length of output: 1648

🏁 Script executed:

# Check how generateRouteRecords is actually called and if there's a higher-level cleanup
rg "generateRouteRecords" packages/router/src/ -B 2 -A 2

Repository: vuejs/router

Length of output: 4860

🏁 Script executed:

# Look at getChildrenSorted to see what happens after delete
rg "getChildrenSorted" packages/router/src/unplugin/core/tree.ts -A 10

Repository: vuejs/router

Length of output: 367

🏁 Script executed:

# Let's check the test file to see if there are test cases for the pruning behavior
cat packages/router/src/unplugin/codegen/generateRouteRecords.spec.ts | head -150

Repository: vuejs/router

Length of output: 5053

🏁 Script executed:

# Look for test snapshots to understand expected behavior
fd "generateRouteRecords.spec.ts.snap" packages/router/

Repository: vuejs/router

Length of output: 140

🏁 Script executed:

# Check if there are any tests specifically for non-matchable nodes or layout wrappers
rg "isMatchable|layout|wrapper" packages/router/src/unplugin/codegen/generateRouteRecords.spec.ts -B 2 -A 5

Repository: vuejs/router

Length of output: 38

🏁 Script executed:

# Let's find the isMatchable method to understand what makes a node matchable
rg "isMatchable" packages/router/src/unplugin/core/tree.ts -A 5

Repository: vuejs/router

Length of output: 273

🏁 Script executed:

# Let's look at the snapshot file to see what behavior is expected
head -200 packages/router/src/unplugin/codegen/__snapshots__/generateRouteRecords.spec.ts.snap

Repository: vuejs/router

Length of output: 3829

🏁 Script executed:

# Check what happens with groups (parentheses in filenames) since they might be non-matchable
rg "group|Group" packages/router/src/unplugin/core/tree.ts -B 2 -A 2

Repository: vuejs/router

Length of output: 459

🏁 Script executed:

# Let's create a test to verify if delete() during forEach on a Map causes issues
python3 << 'EOF'
# Test Map.forEach behavior when deleting during iteration
test_map = {}

class MockNode:
    def __init__(self, name, parent=None):
        self.name = name
        self.parent = parent
        self.deleted = False
    
    def delete(self):
        if self.parent:
            del self.parent[self.name]
        self.deleted = True

# Simulate the scenario
parent_map = {}
for i in range(5):
    parent_map[f'child{i}'] = MockNode(f'child{i}', parent_map)

print("Initial children:", list(parent_map.keys()))

# Simulate forEach with delete (Python dict iteration is equivalent to Map.forEach)
to_delete = []
for key, child in list(parent_map.items()):
    if key in ['child0', 'child2', 'child4']:  # Non-matchable with no children
        to_delete.append(key)

print("Nodes to delete:", to_delete)

# Delete in iteration (problematic)
print("\n--- Problematic: Delete during iteration ---")
test_map1 = {f'child{i}': MockNode(f'child{i}', test_map1 if i == 0 else None) for i in range(5)}
# Can't easily simulate this in Python, but in JS this causes issues

# Safe approach (recommended)
print("\n--- Safe: Collect first, delete after ---")
test_map2 = {f'child{i}': MockNode(f'child{i}') for i in range(5)}
to_delete_safe = [k for k in test_map2.keys() if k in ['child0', 'child2', 'child4']]
for key in to_delete_safe:
    del test_map2[key]
print("Final children:", list(test_map2.keys()))
EOF

Repository: vuejs/router

Length of output: 420

🏁 Script executed:

# Look at the actual git history/blame for this code to see if it was recently added
git log --oneline -20 packages/router/src/unplugin/codegen/generateRouteRecords.ts 2>/dev/null | head -5

Repository: vuejs/router

Length of output: 136

🏁 Script executed:

# Check if there are any bug reports or issues related to this pruning logic
rg "delete.*lone|prune|layout.*wrap" packages/router/ -i --type-list

Repository: vuejs/router

Length of output: 445

🏁 Script executed:

# Get more details about the recent commit that added this pruning logic
git show a07b7fe --stat

Repository: vuejs/router

Length of output: 37058

🏁 Script executed:

# Look at the full commit message and diff
git show a07b7fe

Repository: vuejs/router

Length of output: 50372

Unsafe iteration and incomplete tree pruning during node deletion.

This code has two issues:

  1. Modifying a Map during forEach iteration: Calling child.delete() while iterating over node.children with forEach modifies the parent's Map concurrently, which can cause entries to be skipped. Collect nodes to delete first, then delete in a separate pass.

  2. Cascade pruning doesn't clean parent nodes: This only removes direct leaf nodes. If a parent becomes childless after its children are pruned, it won't be deleted because the parent-level pruning check already occurred before recursing. Example: If A → B → C (all non-matchable), then C gets deleted when processing B, but B itself won't be removed since it had children when checked at A's level.

Safe iteration pattern 
   // delete lone children nodes - they only provide layout wrapping for children
   // so without children they don't make sense to be included in the route records
-  node.children.forEach(child => {
-    if (!child.isMatchable() && child.children.size === 0) {
-      child.delete()
-    }
-  })
+  const toDelete: TreeNode[] = []
+  node.children.forEach(child => {
+    if (!child.isMatchable() && child.children.size === 0) {
+      toDelete.push(child)
+    }
+  })
+  toDelete.forEach(child => child.delete())

The second issue likely requires a different architectural approach—consider post-order traversal pruning or a separate cleanup pass after code generation.

📝 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.

Suggested change
// delete lone children nodes - they only provide layout wrapping for children
// so without children they don't make sense to be included in the route records
node.children.forEach(child => {
if (!child.isMatchable() && child.children.size === 0) {
child.delete()
}
})
// delete lone children nodes - they only provide layout wrapping for children
// so without children they don't make sense to be included in the route records
const toDelete: TreeNode[] = []
node.children.forEach(child => {
if (!child.isMatchable() && child.children.size === 0) {
toDelete.push(child)
}
})
toDelete.forEach(child => child.delete())
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@packages/router/src/unplugin/codegen/generateRouteRecords.ts` around lines 22
- 28, The current loop in generateRouteRecords.ts mutates node.children during
forEach (child.delete()) and won't remove parents that become empty after their
children are pruned; change to a safe post-order pruning: traverse children
first (recursively prune each child), collect children to delete (using
child.isMatchable() and child.children.size === 0 checks after recursive
pruning), then delete them in a separate pass using child.delete();
alternatively implement a separate cleanup pass (pruneNode) that returns whether
a node should be deleted so parent callers can remove now-empty non-matchable
nodes. Ensure references to node.children, child.isMatchable(), and
child.delete() are used in the new post-order logic.


if (node.isRoot()) {
return `[
${node
Expand All @@ -28,12 +36,6 @@ ${node
]`
}

// Skip 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 (!node.isMatchable() && node.children.size === 0) {
return ''
}

const definePageDataList: string[] = []

if (node.hasDefinePage) {
Expand Down