diff --git a/packages/router/__tests__/matcher/pathRanking.spec.ts b/packages/router/__tests__/matcher/pathRanking.spec.ts index 230c3a182..417d3229e 100644 --- a/packages/router/__tests__/matcher/pathRanking.spec.ts +++ b/packages/router/__tests__/matcher/pathRanking.spec.ts @@ -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: [], } ) } diff --git a/packages/router/src/experimental/router.ts b/packages/router/src/experimental/router.ts index a6c70afff..2347a5f9b 100644 --- a/packages/router/src/experimental/router.ts +++ b/packages/router/src/experimental/router.ts @@ -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 + resolver: NEW_RouterResolver } /** @@ -411,14 +412,18 @@ export interface EXPERIMENTAL_RouteRecordRaw extends NEW_MatcherRecordRaw { component?: unknown redirect?: unknown + score: Array } // 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 { /** * Arbitrary data attached to the record. */ meta: RouteMeta + group?: boolean + score: Array } 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) { - 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): 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) } }, } diff --git a/packages/router/src/matcher/pathParserRanker.ts b/packages/router/src/matcher/pathParserRanker.ts index b2c0b40a0..df9bf172e 100644 --- a/packages/router/src/matcher/pathParserRanker.ts +++ b/packages/router/src/matcher/pathParserRanker.ts @@ -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, + b: Pick +): number { let i = 0 const aScore = a.score const bScore = b.score diff --git a/packages/router/src/new-route-resolver/matcher-resolve.spec.ts b/packages/router/src/new-route-resolver/matcher-resolve.spec.ts index 66ca21a89..77b37489d 100644 --- a/packages/router/src/new-route-resolver/matcher-resolve.spec.ts +++ b/packages/router/src/new-route-resolver/matcher-resolve.spec.ts @@ -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, diff --git a/packages/router/src/new-route-resolver/matchers/test-utils.ts b/packages/router/src/new-route-resolver/matchers/test-utils.ts index 250efafd9..4c72d8331 100644 --- a/packages/router/src/new-route-resolver/matchers/test-utils.ts +++ b/packages/router/src/new-route-resolver/matchers/test-utils.ts @@ -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 diff --git a/packages/router/src/new-route-resolver/resolver.spec.ts b/packages/router/src/new-route-resolver/resolver.spec.ts index 436349a04..da7b388e7 100644 --- a/packages/router/src/new-route-resolver/resolver.spec.ts +++ b/packages/router/src/new-route-resolver/resolver.spec.ts @@ -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]], }, ]) diff --git a/packages/router/src/new-route-resolver/resolver.ts b/packages/router/src/new-route-resolver/resolver.ts index 060aee34c..e9d198b0d 100644 --- a/packages/router/src/new-route-resolver/resolver.ts +++ b/packages/router/src/new-route-resolver/resolver.ts @@ -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 } export interface NEW_MatcherRecordBase { @@ -298,9 +301,12 @@ export interface NEW_MatcherRecordBase { query?: MatcherPatternQuery hash?: MatcherPatternHash - group?: boolean - parent?: T + children: T[] + + group?: boolean + aliasOf?: NEW_MatcherRecord + score: Array } /** @@ -352,7 +358,8 @@ export function createCompiledMatcher< records: NEW_MatcherRecordRaw[] = [] ): NEW_RouterResolver { // TODO: we also need an array that has the correct order - const matchers = new Map() + const matcherMap = new Map() + 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['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>( + 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>(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>( + record: T | undefined +): boolean { + while (record) { + if (record.aliasOf) return true + record = record.parent + } + + return false +}