]> git.ipfire.org Git - thirdparty/vuejs/router.git/commitdiff
feat: handle children feat/new-matcher 2415/head
authorEduardo San Martin Morote <posva13@gmail.com>
Thu, 9 Jan 2025 14:47:33 +0000 (15:47 +0100)
committerEduardo San Martin Morote <posva13@gmail.com>
Thu, 9 Jan 2025 14:47:33 +0000 (15:47 +0100)
packages/router/__tests__/matcher/pathRanking.spec.ts
packages/router/src/experimental/router.ts
packages/router/src/matcher/pathParserRanker.ts
packages/router/src/new-route-resolver/matcher-resolve.spec.ts
packages/router/src/new-route-resolver/matchers/test-utils.ts
packages/router/src/new-route-resolver/resolver.spec.ts
packages/router/src/new-route-resolver/resolver.ts

index 230c3a182997c849089453cfc5357106c95c397d..417d3229e05c0a5a9a5ed4da58e84fbd6ea43fc1 100644 (file)
@@ -13,19 +13,9 @@ describe('Path ranking', () => {
       return comparePathParserScore(
         {
           score: a,
-          re: /a/,
-          // @ts-expect-error
-          stringify: v => v,
-          // @ts-expect-error
-          parse: v => v,
-          keys: [],
         },
         {
           score: b,
-          re: /a/,
-          stringify: v => v,
-          parse: v => v,
-          keys: [],
         }
       )
     }
index a6c70afffc690736b00a91d3361c138b0baa40ef..2347a5f9b27ce4e62f698919409459a281f2ff71 100644 (file)
@@ -23,11 +23,12 @@ import {
   type RouterHistory,
 } from '../history/common'
 import type { PathParserOptions } from '../matcher'
-import type {
-  NEW_LocationResolved,
-  NEW_MatcherRecord,
-  NEW_MatcherRecordRaw,
-  NEW_RouterResolver,
+import {
+  type NEW_MatcherRecordBase,
+  type NEW_LocationResolved,
+  type NEW_MatcherRecord,
+  type NEW_MatcherRecordRaw,
+  type NEW_RouterResolver,
 } from '../new-route-resolver/resolver'
 import {
   parseQuery as originalParseQuery,
@@ -194,7 +195,7 @@ export interface EXPERIMENTAL_RouterOptions<
    * Matcher to use to resolve routes.
    * @experimental
    */
-  matcher: NEW_RouterResolver<NEW_MatcherRecordRaw, TMatcherRecord>
+  resolver: NEW_RouterResolver<NEW_MatcherRecordRaw, TMatcherRecord>
 }
 
 /**
@@ -411,14 +412,18 @@ export interface EXPERIMENTAL_RouteRecordRaw extends NEW_MatcherRecordRaw {
   component?: unknown
 
   redirect?: unknown
+  score: Array<number[]>
 }
 
 // TODO: is it worth to have 2 types for the undefined values?
-export interface EXPERIMENTAL_RouteRecordNormalized extends NEW_MatcherRecord {
+export interface EXPERIMENTAL_RouteRecordNormalized
+  extends NEW_MatcherRecordBase<EXPERIMENTAL_RouteRecordNormalized> {
   /**
    * Arbitrary data attached to the record.
    */
   meta: RouteMeta
+  group?: boolean
+  score: Array<number[]>
 }
 
 function normalizeRouteRecord(
@@ -429,6 +434,7 @@ function normalizeRouteRecord(
     name: __DEV__ ? Symbol('anonymous route record') : Symbol(),
     meta: {},
     ...record,
+    children: (record.children || []).map(normalizeRouteRecord),
   }
 }
 
@@ -439,7 +445,7 @@ export function experimental_createRouter(
   EXPERIMENTAL_RouteRecordNormalized
 > {
   const {
-    matcher,
+    resolver,
     parseQuery = originalParseQuery,
     stringifyQuery = originalStringifyQuery,
     history: routerHistory,
@@ -470,11 +476,11 @@ export function experimental_createRouter(
       | EXPERIMENTAL_RouteRecordRaw,
     route?: EXPERIMENTAL_RouteRecordRaw
   ) {
-    let parent: Parameters<(typeof matcher)['addMatcher']>[1] | undefined
+    let parent: Parameters<(typeof resolver)['addMatcher']>[1] | undefined
     let rawRecord: EXPERIMENTAL_RouteRecordRaw
 
     if (isRouteName(parentOrRoute)) {
-      parent = matcher.getMatcher(parentOrRoute)
+      parent = resolver.getMatcher(parentOrRoute)
       if (__DEV__ && !parent) {
         warn(
           `Parent route "${String(
@@ -488,31 +494,31 @@ export function experimental_createRouter(
       rawRecord = parentOrRoute
     }
 
-    const addedRecord = matcher.addMatcher(
+    const addedRecord = resolver.addMatcher(
       normalizeRouteRecord(rawRecord),
       parent
     )
 
     return () => {
-      matcher.removeMatcher(addedRecord)
+      resolver.removeMatcher(addedRecord)
     }
   }
 
   function removeRoute(name: NonNullable<RouteRecordNameGeneric>) {
-    const recordMatcher = matcher.getMatcher(name)
+    const recordMatcher = resolver.getMatcher(name)
     if (recordMatcher) {
-      matcher.removeMatcher(recordMatcher)
+      resolver.removeMatcher(recordMatcher)
     } else if (__DEV__) {
       warn(`Cannot remove non-existent route "${String(name)}"`)
     }
   }
 
   function getRoutes() {
-    return matcher.getMatchers()
+    return resolver.getMatchers()
   }
 
   function hasRoute(name: NonNullable<RouteRecordNameGeneric>): boolean {
-    return !!matcher.getMatcher(name)
+    return !!resolver.getMatcher(name)
   }
 
   function locationAsObject(
@@ -567,7 +573,7 @@ export function experimental_createRouter(
     //   rawLocation.params = targetParams
     // }
 
-    const matchedRoute = matcher.resolve(
+    const matchedRoute = resolver.resolve(
       // incompatible types
       rawLocation as any,
       // incompatible `matched` requires casting
@@ -1226,7 +1232,7 @@ export function experimental_createRouter(
 
     addRoute,
     removeRoute,
-    clearRoutes: matcher.clearMatchers,
+    clearRoutes: resolver.clearMatchers,
     hasRoute,
     getRoutes,
     resolve,
@@ -1307,7 +1313,7 @@ export function experimental_createRouter(
       // TODO: this probably needs to be updated so it can be used by vue-termui
       if ((__DEV__ || __FEATURE_PROD_DEVTOOLS__) && isBrowser) {
         // @ts-expect-error: FIXME: refactor with new types once it's possible
-        addDevtools(app, router, matcher)
+        addDevtools(app, router, resolver)
       }
     },
   }
index b2c0b40a050a7565b4a3a8d8ae51f583feea0adb..df9bf172ea64bb71899233c6875b3a9a8d3af6eb 100644 (file)
@@ -331,7 +331,10 @@ function compareScoreArray(a: number[], b: number[]): number {
  * @param b - second PathParser
  * @returns 0 if both are equal, < 0 if a should be sorted first, > 0 if b
  */
-export function comparePathParserScore(a: PathParser, b: PathParser): number {
+export function comparePathParserScore(
+  a: Pick<PathParser, 'score'>,
+  b: Pick<PathParser, 'score'>
+): number {
   let i = 0
   const aScore = a.score
   const bScore = b.score
index 66ca21a896123dd64e9f266f3a70865d2252b20f..77b37489dbd3258e08f67b22d5918418e952356c 100644 (file)
@@ -42,15 +42,25 @@ function isMatchable(record: RouteRecordRaw): boolean {
   )
 }
 
+function joinPaths(a: string | undefined, b: string) {
+  if (a?.endsWith('/')) {
+    return a + b
+  }
+  return a + '/' + b
+}
+
 function compileRouteRecord(
   record: RouteRecordRaw,
   parentRecord?: RouteRecordRaw
 ): NEW_MatcherRecordRaw {
   // we adapt the path to ensure they are absolute
   // TODO: aliases? they could be handled directly in the path matcher
+  if (!parentRecord && !record.path.startsWith('/')) {
+    throw new Error(`Record without parent must have an absolute path`)
+  }
   const path = record.path.startsWith('/')
     ? record.path
-    : (parentRecord?.path || '') + record.path
+    : joinPaths(parentRecord?.path, record.path)
   record.path = path
   const parser = tokensToParser(
     tokenizePath(record.path),
@@ -62,10 +72,12 @@ function compileRouteRecord(
   return {
     group: !isMatchable(record),
     name: record.name,
+    score: parser.score,
 
     path: {
       match(value) {
         const params = parser.parse(value)
+        // console.log('🌟', parser.re, value, params)
         if (params) {
           return params
         }
@@ -181,20 +193,21 @@ describe('RouterMatcher.resolve', () => {
       : matcher.resolve(
           // FIXME: is this a ts bug?
           // @ts-expect-error
-          typeof fromLocation === 'string'
-            ? { path: fromLocation }
-            : fromLocation
+          fromLocation
         )
 
+    // console.log(matcher.getMatchers())
     // console.log({ toLocation, resolved, expectedLocation, resolvedFrom })
 
     const result = matcher.resolve(
       // FIXME: should work now
       // @ts-expect-error
-      typeof toLocation === 'string' ? { path: toLocation } : toLocation,
+      toLocation,
       resolvedFrom === START_LOCATION ? undefined : resolvedFrom
     )
 
+    // console.log(result)
+
     if (
       expectedLocation.name === undefined ||
       expectedLocation.name !== NO_MATCH_LOCATION.name
@@ -479,7 +492,7 @@ describe('RouterMatcher.resolve', () => {
     // TODO: not sure where this warning should appear now
     it.todo('warns if a path isn not absolute', () => {
       const matcher = createCompiledMatcher([
-        { path: new MatcherPatternPathStatic('/') },
+        { path: new MatcherPatternPathStatic('/'), score: [[80]] },
       ])
       matcher.resolve({ path: 'two' }, matcher.resolve({ path: '/' }))
       expect('received "two"').toHaveBeenWarned()
@@ -1169,26 +1182,34 @@ describe('RouterMatcher.resolve', () => {
     })
   })
 
-  describe.skip('children', () => {
-    const ChildA = { path: 'a', name: 'child-a', components }
-    const ChildB = { path: 'b', name: 'child-b', components }
-    const ChildC = { path: 'c', name: 'child-c', components }
-    const ChildD = { path: '/absolute', name: 'absolute', components }
-    const ChildWithParam = { path: ':p', name: 'child-params', components }
-    const NestedChildWithParam = {
+  describe('children', () => {
+    const ChildA: RouteRecordRaw = { path: 'a', name: 'child-a', components }
+    const ChildB: RouteRecordRaw = { path: 'b', name: 'child-b', components }
+    const ChildC: RouteRecordRaw = { path: 'c', name: 'child-c', components }
+    const ChildD: RouteRecordRaw = {
+      path: '/absolute',
+      name: 'absolute',
+      components,
+    }
+    const ChildWithParam: RouteRecordRaw = {
+      path: ':p',
+      name: 'child-params',
+      components,
+    }
+    const NestedChildWithParam: RouteRecordRaw = {
       ...ChildWithParam,
       name: 'nested-child-params',
     }
-    const NestedChildA = { ...ChildA, name: 'nested-child-a' }
-    const NestedChildB = { ...ChildB, name: 'nested-child-b' }
-    const NestedChildC = { ...ChildC, name: 'nested-child-c' }
-    const Nested = {
+    const NestedChildA: RouteRecordRaw = { ...ChildA, name: 'nested-child-a' }
+    const NestedChildB: RouteRecordRaw = { ...ChildB, name: 'nested-child-b' }
+    const NestedChildC: RouteRecordRaw = { ...ChildC, name: 'nested-child-c' }
+    const Nested: RouteRecordRaw = {
       path: 'nested',
       name: 'nested',
       components,
       children: [NestedChildA, NestedChildB, NestedChildC],
     }
-    const NestedWithParam = {
+    const NestedWithParam: RouteRecordRaw = {
       path: 'nested/:n',
       name: 'nested',
       components,
@@ -1196,7 +1217,7 @@ describe('RouterMatcher.resolve', () => {
     }
 
     it('resolves children', () => {
-      const Foo = {
+      const Foo: RouteRecordRaw = {
         path: '/foo',
         name: 'Foo',
         components,
@@ -1216,8 +1237,8 @@ describe('RouterMatcher.resolve', () => {
     })
 
     it('resolves children with empty paths', () => {
-      const Nested = { path: '', name: 'nested', components }
-      const Foo = {
+      const Nested: RouteRecordRaw = { path: '', name: 'nested', components }
+      const Foo: RouteRecordRaw = {
         path: '/foo',
         name: 'Foo',
         components,
index 250efafd919fcc2d03a92073d9326782e8cac068..4c72d8331a99c6c4d77c2ba27e3118db338f850a 100644 (file)
@@ -68,9 +68,23 @@ export const ANY_HASH_PATTERN_MATCHER: MatcherPatternParams_Base<
 export const EMPTY_PATH_ROUTE = {
   name: 'no params',
   path: EMPTY_PATH_PATTERN_MATCHER,
+  score: [[80]],
+  children: [],
+  parent: undefined,
+} satisfies NEW_MatcherRecord
+
+export const ANY_PATH_ROUTE = {
+  name: 'any path',
+  path: ANY_PATH_PATTERN_MATCHER,
+  score: [[-10]],
+  children: [],
+  parent: undefined,
 } satisfies NEW_MatcherRecord
 
 export const USER_ID_ROUTE = {
   name: 'user-id',
   path: USER_ID_PATH_PATTERN_MATCHER,
+  score: [[80], [70]],
+  children: [],
+  parent: undefined,
 } satisfies NEW_MatcherRecord
index 436349a047cdaa9b7eb9394461d15f14c6839633..da7b388e7a8377713c7ed2fc01151d16ddab30ca 100644 (file)
@@ -11,9 +11,13 @@ import {
   MatcherPatternPathStatic,
   MatcherPatternPathDynamic,
 } from './matcher-pattern'
-import { NEW_MatcherRecord } from './resolver'
 import { miss } from './matchers/errors'
 import { EmptyParams } from './matcher-location'
+import {
+  EMPTY_PATH_ROUTE,
+  USER_ID_ROUTE,
+  ANY_PATH_ROUTE,
+} from './matchers/test-utils'
 
 const ANY_PATH_PATTERN_MATCHER: MatcherPatternPath<{ pathMatch: string }> = {
   match(path) {
@@ -69,27 +73,12 @@ const ANY_HASH_PATTERN_MATCHER: MatcherPatternParams_Base<
   build: ({ hash }) => (hash ? `#${hash}` : ''),
 }
 
-const EMPTY_PATH_ROUTE = {
-  name: 'no params',
-  path: EMPTY_PATH_PATTERN_MATCHER,
-} satisfies NEW_MatcherRecord
-
-const ANY_PATH_ROUTE = {
-  name: 'any path',
-  path: ANY_PATH_PATTERN_MATCHER,
-} satisfies NEW_MatcherRecord
-
-const USER_ID_ROUTE = {
-  name: 'user-id',
-  path: USER_ID_PATH_PATTERN_MATCHER,
-} satisfies NEW_MatcherRecord
-
 describe('RouterMatcher', () => {
   describe('new matchers', () => {
     it('static path', () => {
       const matcher = createCompiledMatcher([
-        { path: new MatcherPatternPathStatic('/') },
-        { path: new MatcherPatternPathStatic('/users') },
+        { path: new MatcherPatternPathStatic('/'), score: [[80]] },
+        { path: new MatcherPatternPathStatic('/users'), score: [[80]] },
       ])
 
       expect(matcher.resolve({ path: '/' })).toMatchObject({
@@ -112,6 +101,7 @@ describe('RouterMatcher', () => {
     it('dynamic path', () => {
       const matcher = createCompiledMatcher([
         {
+          score: [[80], [70]],
           path: new MatcherPatternPathDynamic<{ id: string }>(
             /^\/users\/([^\/]+)$/,
             {
@@ -202,6 +192,7 @@ describe('RouterMatcher', () => {
         const matcher = createCompiledMatcher([
           {
             path: ANY_PATH_PATTERN_MATCHER,
+            score: [[100, -10]],
             query: PAGE_QUERY_PATTERN_MATCHER,
           },
         ])
@@ -220,6 +211,7 @@ describe('RouterMatcher', () => {
       it('resolves string locations with hash', () => {
         const matcher = createCompiledMatcher([
           {
+            score: [[100, -10]],
             path: ANY_PATH_PATTERN_MATCHER,
             hash: ANY_HASH_PATTERN_MATCHER,
           },
@@ -236,6 +228,7 @@ describe('RouterMatcher', () => {
       it('combines path, query and hash params', () => {
         const matcher = createCompiledMatcher([
           {
+            score: [[200, 80], [72]],
             path: USER_ID_PATH_PATTERN_MATCHER,
             query: PAGE_QUERY_PATTERN_MATCHER,
             hash: ANY_HASH_PATTERN_MATCHER,
@@ -253,7 +246,7 @@ describe('RouterMatcher', () => {
     describe('relative locations as strings', () => {
       it('resolves a simple relative location', () => {
         const matcher = createCompiledMatcher([
-          { path: ANY_PATH_PATTERN_MATCHER },
+          { path: ANY_PATH_PATTERN_MATCHER, score: [[-10]] },
         ])
 
         expect(
@@ -311,6 +304,7 @@ describe('RouterMatcher', () => {
           {
             name: 'home',
             path: EMPTY_PATH_PATTERN_MATCHER,
+            score: [[80]],
           },
         ])
 
index 060aee34ce72b25c92b268f95114520a2e970141..e9d198b0daca73d514bdb9cc2e37fd70c1a09ff5 100644 (file)
@@ -25,6 +25,7 @@ import type {
   MatcherParamsFormatted,
 } from './matcher-location'
 import { _RouteRecordProps } from '../typed-routes'
+import { comparePathParserScore } from '../matcher/pathParserRanker'
 
 /**
  * Allowed types for a matcher name.
@@ -286,6 +287,8 @@ export interface NEW_MatcherRecordRaw {
    * Is this a record that groups children. Cannot be matched
    */
   group?: boolean
+
+  score: Array<number[]>
 }
 
 export interface NEW_MatcherRecordBase<T> {
@@ -298,9 +301,12 @@ export interface NEW_MatcherRecordBase<T> {
   query?: MatcherPatternQuery
   hash?: MatcherPatternHash
 
-  group?: boolean
-
   parent?: T
+  children: T[]
+
+  group?: boolean
+  aliasOf?: NEW_MatcherRecord
+  score: Array<number[]>
 }
 
 /**
@@ -352,7 +358,8 @@ export function createCompiledMatcher<
   records: NEW_MatcherRecordRaw[] = []
 ): NEW_RouterResolver<NEW_MatcherRecordRaw, TMatcherRecord> {
   // TODO: we also need an array that has the correct order
-  const matchers = new Map<MatcherName, TMatcherRecord>()
+  const matcherMap = new Map<MatcherName, TMatcherRecord>()
+  const matchers: TMatcherRecord[] = []
 
   // TODO: allow custom encode/decode functions
   // const encodeParams = applyToParams.bind(null, encodeParam)
@@ -424,7 +431,7 @@ export function createCompiledMatcher<
       // either one of them must be defined and is catched by the dev only warn above
       const name = to.name ?? currentLocation?.name
       // FIXME: remove once name cannot be null
-      const matcher = name != null && matchers.get(name)
+      const matcher = name != null && matcherMap.get(name)
       if (!matcher) {
         throw new Error(`Matcher "${String(name)}" not found`)
       }
@@ -474,7 +481,7 @@ export function createCompiledMatcher<
       let matched: NEW_LocationResolved<TMatcherRecord>['matched'] | undefined
       let parsedParams: MatcherParamsFormatted | null | undefined
 
-      for (matcher of matchers.values()) {
+      for (matcher of matchers) {
         // match the path because the path matcher only needs to be matched here
         // match the hash because only the deepest child matters
         // End up by building up the matched array, (reversed so it goes from
@@ -493,6 +500,8 @@ export function createCompiledMatcher<
           // }
 
           parsedParams = { ...pathParams, ...queryParams, ...hashParams }
+          // we found our match!
+          break
         } catch (e) {
           // for debugging tests
           // console.log('❌ ERROR matching', e)
@@ -529,12 +538,23 @@ export function createCompiledMatcher<
       ...record,
       name,
       parent,
+      children: [],
     }
-    // TODO:
-    // record.children
+
+    // insert the matcher if it's matchable
     if (!normalizedRecord.group) {
-      matchers.set(name, normalizedRecord)
+      const index = findInsertionIndex(normalizedRecord, matchers)
+      matchers.splice(index, 0, normalizedRecord)
+      // only add the original record to the name map
+      if (normalizedRecord.name && !isAliasRecord(normalizedRecord))
+        matcherMap.set(normalizedRecord.name, normalizedRecord)
+      // matchers.set(name, normalizedRecord)
     }
+
+    record.children?.forEach(childRecord =>
+      normalizedRecord.children.push(addMatcher(childRecord, normalizedRecord))
+    )
+
     return normalizedRecord
   }
 
@@ -543,20 +563,25 @@ export function createCompiledMatcher<
   }
 
   function removeMatcher(matcher: TMatcherRecord) {
-    matchers.delete(matcher.name)
+    matcherMap.delete(matcher.name)
+    for (const child of matcher.children) {
+      removeMatcher(child)
+    }
+    // TODO: delete from matchers
     // TODO: delete children and aliases
   }
 
   function clearMatchers() {
-    matchers.clear()
+    matchers.splice(0, matchers.length)
+    matcherMap.clear()
   }
 
   function getMatchers() {
-    return Array.from(matchers.values())
+    return matchers
   }
 
   function getMatcher(name: MatcherName) {
-    return matchers.get(name)
+    return matcherMap.get(name)
   }
 
   return {
@@ -569,3 +594,76 @@ export function createCompiledMatcher<
     getMatchers,
   }
 }
+
+/**
+ * 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 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
+ */
+function findInsertionIndex<T extends NEW_MatcherRecordBase<T>>(
+  matcher: T,
+  matchers: T[]
+) {
+  // 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])
+
+    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 < 0) {
+      // This should never happen
+      warn(
+        // TODO: fix stringifying new matchers
+        `Finding ancestor route "${insertionAncestor.path}" failed for "${matcher.path}"`
+      )
+    }
+  }
+
+  return upper
+}
+
+function getInsertionAncestor<T extends NEW_MatcherRecordBase<T>>(matcher: T) {
+  let ancestor: T | undefined = matcher
+
+  while ((ancestor = ancestor.parent)) {
+    if (!ancestor.group && comparePathParserScore(matcher, ancestor) === 0) {
+      return ancestor
+    }
+  }
+
+  return
+}
+
+/**
+ * Checks if a record or any of its parent is an alias
+ * @param record
+ */
+function isAliasRecord<T extends NEW_MatcherRecordBase<T>>(
+  record: T | undefined
+): boolean {
+  while (record) {
+    if (record.aliasOf) return true
+    record = record.parent
+  }
+
+  return false
+}