Fix matcher ordering in edge cases

Author: skirtles-code

packages/router/src/matcher/index.ts

@@ -158,6 +158,12 @@ export function createRouterMatcher(
           removeRoute(record.name)
       }
 
+      // Avoid adding a record that doesn't display anything. This allows passing through records without a component to
+      // not be reached and pass through the catch all route
+      if (isMatchable(matcher)) {
+        insertMatcher(matcher)
+      }
+
       if (mainNormalizedRecord.children) {
         const children = mainNormalizedRecord.children
         for (let i = 0; i < children.length; i++) {
@@ -177,17 +183,6 @@ export function createRouterMatcher(
       // if (parent && isAliasRecord(originalRecord)) {
       //   parent.children.push(originalRecord)
       // }
-
-      // Avoid adding a record that doesn't display anything. This allows passing through records without a component to
-      // not be reached and pass through the catch all route
-      if (
-        (matcher.record.components &&
-          Object.keys(matcher.record.components).length) ||
-        matcher.record.name ||
-        matcher.record.redirect
-      ) {
-        insertMatcher(matcher)
-      }
     }
 
     return originalMatcher
@@ -514,8 +509,8 @@ function checkMissingParamsInAbsolutePath(
 /**
  * Performs a binary search to find the correct insertion index for a new matcher.
  *
- * Matchers are primarily sorted by their score. If scores are tied then the matcher's depth is used instead.
- * The depth check ensures that a child with an empty path comes before its parent.
+ * Matchers are primarily sorted by their score. If scores are tied then we also consider parent/child relationships,
+ * with descendants coming before ancestors. If there's still a tie, new routes are inserted after existing routes.
  *
  * @param matcher - new matcher to be inserted
  * @param matchers - existing matchers
@@ -524,38 +519,59 @@ function findInsertionIndex(
   matcher: RouteRecordMatcher,
   matchers: RouteRecordMatcher[]
 ) {
-  const depth = getDepth(matcher)
-
+  // First phase: binary search based on score
   let lower = 0
   let upper = matchers.length
 
   while (lower !== upper) {
     const mid = (lower + upper) >> 1
-    const sortOrder =
-      comparePathParserScore(matcher, matchers[mid]) ||
-      getDepth(matchers[mid]) - depth
+    const sortOrder = comparePathParserScore(matcher, matchers[mid])
 
-    if (sortOrder === 0) {
-      return mid
-    } else if (sortOrder < 0) {
+    if (sortOrder < 0) {
       upper = mid
     } else {
       lower = mid + 1
     }
   }
 
+  // Second phase: check for an ancestor with the same score
+  const insertionAncestor = getInsertionAncestor(matcher)
+
+  if (insertionAncestor) {
+    upper = matchers.lastIndexOf(insertionAncestor, upper - 1)
+
+    if (__DEV__ && upper === -1) {
+      // This should never happen
+      warn(
+        `Finding ancestor route ${insertionAncestor.record.path} failed for ${matcher.record.path}`
+      )
+    }
+  }
+
   return upper
 }
 
-function getDepth(record: RouteRecordMatcher) {
-  let depth = 0
+function getInsertionAncestor(matcher: RouteRecordMatcher) {
+  let ancestor: RouteRecordMatcher | undefined = matcher
 
-  while (record.parent) {
-    ++depth
-    record = record.parent
+  while ((ancestor = ancestor.parent)) {
+    if (
+      isMatchable(ancestor) &&
+      comparePathParserScore(matcher, ancestor) === 0
+    ) {
+      return ancestor
+    }
   }
 
-  return depth
+  return
+}
+
+function isMatchable({ record }: RouteRecordMatcher): boolean {
+  return !!(
+    record.name ||
+    (record.components && Object.keys(record.components).length) ||
+    record.redirect
+  )
 }
 
 export type { PathParserOptions, _PathParserOptions }