From 1ca2a13808b5d80020aac065529bdfe5c94fdded Mon Sep 17 00:00:00 2001 From: Eduardo San Martin Morote Date: Mon, 9 Dec 2024 21:57:13 +0100 Subject: [PATCH] refactor: simplify matcher interfaces --- .../new-route-resolver/matcher-location.ts | 6 + .../src/new-route-resolver/matcher.spec.ts | 250 +++++++++--------- .../src/new-route-resolver/matcher.test-d.ts | 4 +- .../router/src/new-route-resolver/matcher.ts | 143 +++++++--- .../new-route-resolver/new-matcher-pattern.ts | 197 ++++++++++++++ packages/router/src/query.ts | 3 +- 6 files changed, 438 insertions(+), 165 deletions(-) create mode 100644 packages/router/src/new-route-resolver/new-matcher-pattern.ts diff --git a/packages/router/src/new-route-resolver/matcher-location.ts b/packages/router/src/new-route-resolver/matcher-location.ts index c205a4564..3744e8cec 100644 --- a/packages/router/src/new-route-resolver/matcher-location.ts +++ b/packages/router/src/new-route-resolver/matcher-location.ts @@ -6,8 +6,14 @@ import type { MatcherName } from './matcher' */ export type MatcherParamsFormatted = Record +/** + * Empty object in TS. + */ +export type EmptyParams = Record + export interface MatcherLocationAsNamed { name: MatcherName + // FIXME: should this be optional? params: MatcherParamsFormatted query?: LocationQueryRaw hash?: string diff --git a/packages/router/src/new-route-resolver/matcher.spec.ts b/packages/router/src/new-route-resolver/matcher.spec.ts index 52d8b208a..3cb67af19 100644 --- a/packages/router/src/new-route-resolver/matcher.spec.ts +++ b/packages/router/src/new-route-resolver/matcher.spec.ts @@ -1,6 +1,14 @@ import { describe, expect, it } from 'vitest' -import { MatcherPatternImpl, MatcherPatternPath } from './matcher-pattern' -import { createCompiledMatcher } from './matcher' +import { MatcherPatternImpl } from './matcher-pattern' +import { createCompiledMatcher, NO_MATCH_LOCATION } from './matcher' +import { + MatcherPatternParams_Base, + MatcherPattern, + MatcherPatternPath, + MatcherPatternQuery, +} from './new-matcher-pattern' +import { miss } from './matchers/errors' +import { EmptyParams } from './matcher-location' function createMatcherPattern( ...args: ConstructorParameters @@ -8,54 +16,121 @@ function createMatcherPattern( return new MatcherPatternImpl(...args) } -const EMPTY_PATH_PATTERN_MATCHER = { - match: (path: string) => ({}), - parse: (params: {}) => ({}), - serialize: (params: {}) => ({}), - buildPath: () => '/', -} satisfies MatcherPatternPath +const ANY_PATH_PATTERN_MATCHER: MatcherPatternPath<{ pathMatch: string }> = { + match(path) { + return { pathMatch: path } + }, + build({ pathMatch }) { + return pathMatch + }, +} + +const EMPTY_PATH_PATTERN_MATCHER: MatcherPatternPath = { + match: path => { + if (path !== '/') { + throw miss() + } + return {} + }, + build: () => '/', +} + +const USER_ID_PATH_PATTERN_MATCHER: MatcherPatternPath<{ id: number }> = { + match(value) { + const match = value.match(/^\/users\/(\d+)$/) + if (!match?.[1]) { + throw miss() + } + const id = Number(match[1]) + if (Number.isNaN(id)) { + throw miss() + } + return { id } + }, + build({ id }) { + return `/users/${id}` + }, +} + +const PAGE_QUERY_PATTERN_MATCHER: MatcherPatternQuery<{ page: number }> = { + match: query => { + const page = Number(query.page) + return { + page: Number.isNaN(page) ? 1 : page, + } + }, + build: params => ({ page: String(params.page) }), +} satisfies MatcherPatternQuery<{ page: number }> + +const ANY_HASH_PATTERN_MATCHER: MatcherPatternParams_Base< + string, + { hash: string | null } +> = { + match: hash => ({ hash: hash ? hash.slice(1) : null }), + build: ({ hash }) => (hash ? `#${hash}` : ''), +} + +const EMPTY_PATH_ROUTE = { + name: 'no params', + path: EMPTY_PATH_PATTERN_MATCHER, +} satisfies MatcherPattern + +const USER_ID_ROUTE = { + name: 'user-id', + path: USER_ID_PATH_PATTERN_MATCHER, +} satisfies MatcherPattern describe('Matcher', () => { + describe('adding and removing', () => { + it('add static path', () => { + const matcher = createCompiledMatcher() + matcher.addRoute(EMPTY_PATH_ROUTE) + }) + + it('adds dynamic path', () => { + const matcher = createCompiledMatcher() + matcher.addRoute(USER_ID_ROUTE) + }) + }) + describe('resolve()', () => { describe('absolute locationss as strings', () => { it('resolves string locations with no params', () => { const matcher = createCompiledMatcher() - matcher.addRoute( - createMatcherPattern(Symbol('foo'), EMPTY_PATH_PATTERN_MATCHER) - ) + matcher.addRoute(EMPTY_PATH_ROUTE) - expect(matcher.resolve('/foo?a=a&b=b#h')).toMatchObject({ - path: '/foo', + expect(matcher.resolve('/?a=a&b=b#h')).toMatchObject({ + path: '/', params: {}, query: { a: 'a', b: 'b' }, hash: '#h', }) }) + it('resolves a not found string', () => { + const matcher = createCompiledMatcher() + expect(matcher.resolve('/bar?q=1#hash')).toEqual({ + ...NO_MATCH_LOCATION, + fullPath: '/bar?q=1#hash', + path: '/bar', + query: { q: '1' }, + hash: '#hash', + matched: [], + }) + }) + it('resolves string locations with params', () => { const matcher = createCompiledMatcher() - matcher.addRoute( - // /users/:id - createMatcherPattern(Symbol('foo'), { - match: (path: string) => { - const match = path.match(/^\/foo\/([^/]+?)$/) - if (!match) throw new Error('no match') - return { id: match[1] } - }, - parse: (params: { id: string }) => ({ id: Number(params.id) }), - serialize: (params: { id: number }) => ({ id: String(params.id) }), - buildPath: params => `/foo/${params.id}`, - }) - ) - - expect(matcher.resolve('/foo/1?a=a&b=b#h')).toMatchObject({ - path: '/foo/1', + matcher.addRoute(USER_ID_ROUTE) + + expect(matcher.resolve('/users/1?a=a&b=b#h')).toMatchObject({ + path: '/users/1', params: { id: 1 }, query: { a: 'a', b: 'b' }, hash: '#h', }) - expect(matcher.resolve('/foo/54?a=a&b=b#h')).toMatchObject({ - path: '/foo/54', + expect(matcher.resolve('/users/54?a=a&b=b#h')).toMatchObject({ + path: '/users/54', params: { id: 54 }, query: { a: 'a', b: 'b' }, hash: '#h', @@ -64,21 +139,16 @@ describe('Matcher', () => { it('resolve string locations with query', () => { const matcher = createCompiledMatcher() - matcher.addRoute( - createMatcherPattern(Symbol('foo'), EMPTY_PATH_PATTERN_MATCHER, { - match: query => ({ - id: Array.isArray(query.id) ? query.id[0] : query.id, - }), - parse: (params: { id: string }) => ({ id: Number(params.id) }), - serialize: (params: { id: number }) => ({ id: String(params.id) }), - }) - ) - - expect(matcher.resolve('/foo?id=100&b=b#h')).toMatchObject({ - params: { id: 100 }, + matcher.addRoute({ + path: ANY_PATH_PATTERN_MATCHER, + query: PAGE_QUERY_PATTERN_MATCHER, + }) + + expect(matcher.resolve('/foo?page=100&b=b#h')).toMatchObject({ + params: { page: 100 }, path: '/foo', query: { - id: '100', + page: '100', b: 'b', }, hash: '#h', @@ -87,84 +157,29 @@ describe('Matcher', () => { it('resolves string locations with hash', () => { const matcher = createCompiledMatcher() - matcher.addRoute( - createMatcherPattern( - Symbol('foo'), - EMPTY_PATH_PATTERN_MATCHER, - undefined, - { - match: hash => hash, - parse: hash => ({ a: hash.slice(1) }), - serialize: ({ a }) => '#a', - } - ) - ) + matcher.addRoute({ + path: ANY_PATH_PATTERN_MATCHER, + hash: ANY_HASH_PATTERN_MATCHER, + }) expect(matcher.resolve('/foo?a=a&b=b#bar')).toMatchObject({ hash: '#bar', - params: { a: 'bar' }, + params: { hash: 'bar' }, path: '/foo', query: { a: 'a', b: 'b' }, }) }) - it('returns a valid location with an empty `matched` array if no match', () => { + it('combines path, query and hash params', () => { const matcher = createCompiledMatcher() - expect(matcher.resolve('/bar')).toMatchInlineSnapshot( - { - hash: '', - matched: [], - params: {}, - path: '/bar', - query: {}, - }, - ` - { - "fullPath": "/bar", - "hash": "", - "matched": [], - "name": Symbol(no-match), - "params": {}, - "path": "/bar", - "query": {}, - } - ` - ) - }) + matcher.addRoute({ + path: USER_ID_PATH_PATTERN_MATCHER, + query: PAGE_QUERY_PATTERN_MATCHER, + hash: ANY_HASH_PATTERN_MATCHER, + }) - it('resolves string locations with all', () => { - const matcher = createCompiledMatcher() - matcher.addRoute( - createMatcherPattern( - Symbol('foo'), - { - buildPath: params => `/foo/${params.id}`, - match: path => { - const match = path.match(/^\/foo\/([^/]+?)$/) - if (!match) throw new Error('no match') - return { id: match[1] } - }, - parse: params => ({ id: Number(params.id) }), - serialize: params => ({ id: String(params.id) }), - }, - { - match: query => ({ - id: Array.isArray(query.id) ? query.id[0] : query.id, - }), - parse: params => ({ q: Number(params.id) }), - serialize: params => ({ id: String(params.q) }), - }, - { - match: hash => hash, - parse: hash => ({ a: hash.slice(1) }), - serialize: ({ a }) => '#a', - } - ) - ) - - expect(matcher.resolve('/foo/1?id=100#bar')).toMatchObject({ - hash: '#bar', - params: { id: 1, q: 100, a: 'bar' }, + expect(matcher.resolve('/users/24?page=100#bar')).toMatchObject({ + params: { id: 24, page: 100, hash: 'bar' }, }) }) }) @@ -172,9 +187,7 @@ describe('Matcher', () => { describe('relative locations as strings', () => { it('resolves a simple relative location', () => { const matcher = createCompiledMatcher() - matcher.addRoute( - createMatcherPattern(Symbol('foo'), EMPTY_PATH_PATTERN_MATCHER) - ) + matcher.addRoute({ path: ANY_PATH_PATTERN_MATCHER }) expect( matcher.resolve('foo', matcher.resolve('/nested/')) @@ -206,9 +219,10 @@ describe('Matcher', () => { describe('named locations', () => { it('resolves named locations with no params', () => { const matcher = createCompiledMatcher() - matcher.addRoute( - createMatcherPattern('home', EMPTY_PATH_PATTERN_MATCHER) - ) + matcher.addRoute({ + name: 'home', + path: EMPTY_PATH_PATTERN_MATCHER, + }) expect(matcher.resolve({ name: 'home', params: {} })).toMatchObject({ name: 'home', diff --git a/packages/router/src/new-route-resolver/matcher.test-d.ts b/packages/router/src/new-route-resolver/matcher.test-d.ts index bb45c5129..c50731a1e 100644 --- a/packages/router/src/new-route-resolver/matcher.test-d.ts +++ b/packages/router/src/new-route-resolver/matcher.test-d.ts @@ -1,8 +1,8 @@ import { describe, expectTypeOf, it } from 'vitest' -import { NEW_LocationResolved, createCompiledMatcher } from './matcher' +import { NEW_LocationResolved, RouteResolver } from './matcher' describe('Matcher', () => { - const matcher = createCompiledMatcher() + const matcher: RouteResolver = {} as any describe('matcher.resolve()', () => { it('resolves absolute string locations', () => { diff --git a/packages/router/src/new-route-resolver/matcher.ts b/packages/router/src/new-route-resolver/matcher.ts index 31b1d0319..c6af61e98 100644 --- a/packages/router/src/new-route-resolver/matcher.ts +++ b/packages/router/src/new-route-resolver/matcher.ts @@ -4,7 +4,12 @@ import { normalizeQuery, stringifyQuery, } from '../query' -import type { MatcherPattern } from './matcher-pattern' +import type { + MatcherPattern, + MatcherPatternHash, + MatcherPatternPath, + MatcherPatternQuery, +} from './new-matcher-pattern' import { warn } from '../warning' import { SLASH_RE, @@ -17,13 +22,17 @@ import type { MatcherLocationAsRelative, MatcherParamsFormatted, } from './matcher-location' +import { RouteRecordRaw } from 'test-dts' +/** + * Allowed types for a matcher name. + */ export type MatcherName = string | symbol /** * Manage and resolve routes. Also handles the encoding, decoding, parsing and serialization of params, query, and hash. */ -export interface RouteResolver { +export interface RouteResolver { /** * Resolves an absolute location (like `/path/to/somewhere`). */ @@ -59,8 +68,8 @@ export interface RouteResolver { currentLocation: NEW_LocationResolved ): NEW_LocationResolved - addRoute(matcher: MatcherPattern, parent?: MatcherPattern): void - removeRoute(matcher: MatcherPattern): void + addRoute(matcher: Matcher, parent?: MatcherNormalized): MatcherNormalized + removeRoute(matcher: MatcherNormalized): void clearRoutes(): void } @@ -204,7 +213,39 @@ export const NO_MATCH_LOCATION = { matched: [], } satisfies Omit -export function createCompiledMatcher(): RouteResolver { +// FIXME: later on, the MatcherRecord should be compatible with RouteRecordRaw (which can miss a path, have children, etc) + +export interface MatcherRecordRaw { + name?: MatcherName + + path: MatcherPatternPath + + query?: MatcherPatternQuery + + hash?: MatcherPatternHash + + children?: MatcherRecordRaw[] +} + +// const a: RouteRecordRaw = {} as any + +/** + * Build the `matched` array of a record that includes all parent records from the root to the current one. + */ +function buildMatched(record: MatcherPattern): MatcherPattern[] { + const matched: MatcherPattern[] = [] + let node: MatcherPattern | undefined = record + while (node) { + matched.unshift(node) + node = node.parent + } + return matched +} + +export function createCompiledMatcher(): RouteResolver< + MatcherRecordRaw, + MatcherPattern +> { const matchers = new Map() // TODO: allow custom encode/decode functions @@ -225,23 +266,39 @@ export function createCompiledMatcher(): RouteResolver { const url = parseURL(parseQuery, location, currentLocation?.path) let matcher: MatcherPattern | undefined + let matched: NEW_LocationResolved['matched'] | undefined let parsedParams: MatcherParamsFormatted | null | undefined for (matcher of matchers.values()) { - const params = matcher.matchLocation(url) - if (params) { - parsedParams = matcher.parseParams( - transformObject(String, decode, params[0]), - // already decoded - params[1], - params[2] + // 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 + // root to child) and then match and merge all queries + try { + const pathParams = matcher.path.match(url.path) + const hashParams = matcher.hash?.match(url.hash) + matched = buildMatched(matcher) + const queryParams: MatcherQueryParams = Object.assign( + {}, + ...matched.map(matcher => matcher.query?.match(url.query)) ) + // TODO: test performance + // for (const matcher of matched) { + // Object.assign(queryParams, matcher.query?.match(url.query)) + // } + + parsedParams = { ...pathParams, ...queryParams, ...hashParams } + // console.log('parsedParams', parsedParams) + if (parsedParams) break + } catch (e) { + // for debugging tests + // console.log('❌ ERROR matching', e) } } // No match location - if (!parsedParams || !matcher) { + if (!parsedParams || !matched) { return { ...url, ...NO_MATCH_LOCATION, @@ -253,12 +310,13 @@ export function createCompiledMatcher(): RouteResolver { return { ...url, - name: matcher.name, + // matcher exists if matched exists + name: matcher!.name, params: parsedParams, // already decoded query: url.query, hash: url.hash, - matched: [], + matched, } } else { // relative location or by name @@ -284,46 +342,43 @@ export function createCompiledMatcher(): RouteResolver { } // unencoded params in a formatted form that the user came up with - const params: MatcherParamsFormatted = - location.params ?? currentLocation!.params - const mixedUnencodedParams = matcher.matchParams(params) - - if (!mixedUnencodedParams) { - throw new Error( - `Invalid params for matcher "${String(name)}":\n${JSON.stringify( - params, - null, - 2 - )}` - ) + const params: MatcherParamsFormatted = { + ...currentLocation?.params, + ...location.params, } - - const path = matcher.buildPath( - // encode the values before building the path - transformObject(String, encodeParam, mixedUnencodedParams[0]) + const path = matcher.path.build(params) + const hash = matcher.hash?.build(params) ?? '' + const matched = buildMatched(matcher) + const query = Object.assign( + { + ...currentLocation?.query, + ...normalizeQuery(location.query), + }, + ...matched.map(matcher => matcher.query?.build(params)) ) - // TODO: should pick query from the params but also from the location and merge them - const query = { - ...normalizeQuery(location.query), - // ...matcher.extractQuery(mixedUnencodedParams[1]) - } - const hash = mixedUnencodedParams[2] ?? location.hash ?? '' - return { name, - fullPath: stringifyURL(stringifyQuery, { path, query: {}, hash }), + fullPath: stringifyURL(stringifyQuery, { path, query, hash }), path, - params, - hash, query, - matched: [], + hash, + params, + matched, } } } - function addRoute(matcher: MatcherPattern, parent?: MatcherPattern) { - matchers.set(matcher.name, matcher) + function addRoute(record: MatcherRecordRaw, parent?: MatcherPattern) { + const name = record.name ?? (__DEV__ ? Symbol('unnamed-route') : Symbol()) + // FIXME: proper normalization of the record + const normalizedRecord: MatcherPattern = { + ...record, + name, + parent, + } + matchers.set(name, normalizedRecord) + return normalizedRecord } function removeRoute(matcher: MatcherPattern) { diff --git a/packages/router/src/new-route-resolver/new-matcher-pattern.ts b/packages/router/src/new-route-resolver/new-matcher-pattern.ts new file mode 100644 index 000000000..f231490cd --- /dev/null +++ b/packages/router/src/new-route-resolver/new-matcher-pattern.ts @@ -0,0 +1,197 @@ +import { MatcherName, MatcherQueryParams } from './matcher' +import { EmptyParams, MatcherParamsFormatted } from './matcher-location' +import { MatchMiss, miss } from './matchers/errors' + +export interface MatcherLocation { + /** + * Encoded path + */ + path: string + + /** + * Decoded query. + */ + query: MatcherQueryParams + + /** + * Decoded hash. + */ + hash: string +} + +export interface OLD_MatcherPattern { + /** + * Name of the matcher. Unique across all matchers. + */ + name: MatcherName + + match(location: MatcherLocation): TParams | null + + toLocation(params: TParams): MatcherLocation +} + +export interface MatcherPattern { + /** + * Name of the matcher. Unique across all matchers. + */ + name: MatcherName + + path: MatcherPatternPath + query?: MatcherPatternQuery + hash?: MatcherPatternHash + + parent?: MatcherPattern +} + +export interface MatcherPatternParams_Base< + TIn = string, + TOut extends MatcherParamsFormatted = MatcherParamsFormatted +> { + match(value: TIn): TOut + + build(params: TOut): TIn + // get: (value: MatcherQueryParamsValue) => T + // set?: (value: T) => MatcherQueryParamsValue + // default?: T | (() => T) +} + +export interface MatcherPatternPath< + TParams extends MatcherParamsFormatted = MatcherParamsFormatted +> extends MatcherPatternParams_Base {} + +export class MatcherPatternPathStatic + implements MatcherPatternPath +{ + constructor(private path: string) {} + + match(path: string): EmptyParams { + if (path !== this.path) { + throw miss() + } + return {} + } + + build(): string { + return this.path + } +} +// example of a static matcher built at runtime +// new MatcherPatternPathStatic('/') + +// example of a generated matcher at build time +const HomePathMatcher = { + match: path => { + if (path !== '/') { + throw miss() + } + return {} + }, + build: () => '/', +} satisfies MatcherPatternPath + +export interface MatcherPatternQuery< + TParams extends MatcherParamsFormatted = MatcherParamsFormatted +> extends MatcherPatternParams_Base {} + +const PaginationQueryMatcher = { + match: query => { + const page = Number(query.page) + return { + page: Number.isNaN(page) ? 1 : page, + } + }, + build: params => ({ page: String(params.page) }), +} satisfies MatcherPatternQuery<{ page: number }> + +export interface MatcherPatternHash< + TParams extends MatcherParamsFormatted = MatcherParamsFormatted +> extends MatcherPatternParams_Base {} + +const HeaderHashMatcher = { + match: hash => + hash.startsWith('#') + ? { + header: hash.slice(1), + } + : {}, // null also works + build: ({ header }) => (header ? `#${header}` : ''), +} satisfies MatcherPatternHash<{ header?: string }> + +export class MatcherPatternImpl< + PathParams extends MatcherParamsFormatted, + QueryParams extends MatcherParamsFormatted = EmptyParams, + HashParams extends MatcherParamsFormatted = EmptyParams +> implements OLD_MatcherPattern +{ + parent: MatcherPatternImpl | null = null + children: MatcherPatternImpl[] = [] + + constructor( + public name: MatcherName, + private path: MatcherPatternPath, + private query?: MatcherPatternQuery, + private hash?: MatcherPatternHash + ) {} + + /** + * Matches a parsed query against the matcher and all of the parents. + * @param query - query to match + * @returns matched + * @throws {MatchMiss} if the query does not match + */ + queryMatch(query: MatcherQueryParams): QParams { + // const queryParams: QParams = {} as QParams + const queryParams: QParams[] = [] + let current: MatcherPatternImpl< + MatcherParamsFormatted, + MatcherParamsFormatted, + MatcherParamsFormatted + > | null = this + + while (current) { + queryParams.push(current.query?.match(query) as QParams) + current = current.parent + } + // we give the later matchers precedence + return Object.assign({}, ...queryParams.reverse()) + } + + queryBuild(params: QParams): MatcherQueryParams { + const query: MatcherQueryParams = {} + let current: MatcherPatternImpl< + MatcherParamsFormatted, + MatcherParamsFormatted, + MatcherParamsFormatted + > | null = this + while (current) { + Object.assign(query, current.query?.build(params)) + current = current.parent + } + return query + } + + match( + location: MatcherLocation + ): (PathParams & QParams & HashParams) | null { + try { + const pathParams = this.path.match(location.path) + const queryParams = this.queryMatch(location.query) + const hashParams = this.hash?.match(location.hash) ?? ({} as HashParams) + + return { ...pathParams, ...queryParams, ...hashParams } + } catch (err) {} + + return null + } + + toLocation(params: PathParams & QueryParams & HashParams): MatcherLocation { + return { + path: this.path.build(params), + query: this.query?.build(params) ?? {}, + hash: this.hash?.build(params) ?? '', + } + } +} + +// const matcher = new MatcherPatternImpl('name', HomePathMatcher, PaginationQueryMatcher, HeaderHashMatcher) +// matcher.match({ path: '/', query: {}, hash: '' })!.page diff --git a/packages/router/src/query.ts b/packages/router/src/query.ts index 75a8cc70b..55e77c714 100644 --- a/packages/router/src/query.ts +++ b/packages/router/src/query.ts @@ -89,9 +89,10 @@ export function parseQuery(search: string): LocationQuery { * @param query - query object to stringify * @returns string version of the query without the leading `?` */ -export function stringifyQuery(query: LocationQueryRaw): string { +export function stringifyQuery(query: LocationQueryRaw | undefined): string { let search = '' for (let key in query) { + // FIXME: we could do search ||= '?' so that the returned value already has the leading ? const value = query[key] key = encodeQueryKey(key) if (value == null) {