Skip to content

Commit

Permalink
Fix matcher ordering in edge cases
Browse files Browse the repository at this point in the history
  • Loading branch information
skirtles-code committed Feb 24, 2024
1 parent b894ef7 commit 48db602
Showing 1 changed file with 43 additions and 27 deletions.
70 changes: 43 additions & 27 deletions packages/router/src/matcher/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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++) {
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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 }

0 comments on commit 48db602

Please sign in to comment.