refactor: simplify param parsers and allow trailingSlash in dynamic params

Author: posva

packages/router/src/experimental/route-resolver/matchers/matcher-pattern.spec.ts

@@ -132,7 +132,7 @@ describe('MatcherPatternPathDynamic', () => {
       /^\/teams\/([^/]+?)\/b$/i,
       {
         // all defaults
-        teamId: {},
+        teamId: [{}],
       },
       ['teams', 1, 'b']
     )
@@ -153,7 +153,7 @@ describe('MatcherPatternPathDynamic', () => {
     const pattern = new MatcherPatternPathDynamic(
       /^\/teams\/([^/]+?)$/i,
       {
-        teamId: {},
+        teamId: [{}],
       },
       ['teams', 1]
     )
@@ -165,7 +165,7 @@ describe('MatcherPatternPathDynamic', () => {
     const pattern = new MatcherPatternPathDynamic(
       /^\/teams(?:\/([^/]+?))?\/b$/i,
       {
-        teamId: {},
+        teamId: [{}, false, true],
       },
       ['teams', 1, 'b']
     )
@@ -180,30 +180,30 @@ describe('MatcherPatternPathDynamic', () => {
     expect(pattern.build({ teamId: '' })).toBe('/teams/b')
   })
 
-  it.todo('optional param in the end', () => {
+  it('optional param in the end', () => {
     const pattern = new MatcherPatternPathDynamic(
-      /^\/teams(?:\/([^/]+?))?\/b$/i,
+      /^\/teams(?:\/([^/]+?))?$/i,
       {
-        teamId: {},
+        teamId: [{}, false, true],
       },
-      ['teams', 1, 'b']
+      ['teams', 1]
     )
 
-    expect(pattern.match('/teams/b')).toEqual({ teamId: null })
-    expect(pattern.match('/teams/123/b')).toEqual({ teamId: '123' })
+    expect(pattern.match('/teams')).toEqual({ teamId: null })
+    expect(() => pattern.match('/teams/')).toThrow()
+    expect(pattern.match('/teams/123')).toEqual({ teamId: '123' })
     expect(() => pattern.match('/teams/123/c')).toThrow()
-    expect(() => pattern.match('/teams/123/b/c')).toThrow()
     expect(() => pattern.match('/teams//b')).toThrow()
-    expect(pattern.build({ teamId: '123' })).toBe('/teams/123/b')
-    expect(pattern.build({ teamId: null })).toBe('/teams/b')
-    expect(pattern.build({ teamId: '' })).toBe('/teams/b')
+    expect(pattern.build({ teamId: '123' })).toBe('/teams/123')
+    expect(pattern.build({ teamId: null })).toBe('/teams')
+    expect(pattern.build({ teamId: '' })).toBe('/teams')
   })
 
   it('repeatable param', () => {
     const pattern = new MatcherPatternPathDynamic(
       /^\/teams\/(.+?)\/b$/i,
       {
-        teamId: { repeat: true },
+        teamId: [{}, true],
       },
       ['teams', 1, 'b']
     )
@@ -218,6 +218,25 @@ describe('MatcherPatternPathDynamic', () => {
     expect(pattern.build({ teamId: ['123', '456'] })).toBe('/teams/123/456/b')
   })
 
+  it('repeatable param in the end', () => {
+    const pattern = new MatcherPatternPathDynamic(
+      /^\/teams\/(.+?)$/i,
+      {
+        teamId: [{}, true],
+      },
+      ['teams', 1]
+    )
+
+    expect(pattern.match('/teams/123')).toEqual({ teamId: ['123'] })
+    expect(pattern.match('/teams/123/456')).toEqual({ teamId: ['123', '456'] })
+    expect(() => pattern.match('/teams')).toThrow()
+    expect(() => pattern.match('/teams/')).toThrow()
+    expect(() => pattern.match('/teams/123/')).toThrow()
+    expect(pattern.build({ teamId: ['123'] })).toBe('/teams/123')
+    expect(pattern.build({ teamId: ['123', '456'] })).toBe('/teams/123/456')
+    expect(() => pattern.build({ teamId: [] })).toThrow()
+  })
+
   it.todo('catch all route', () => {
     // const pattern = new MatcherPatternPathDynamic(
   })
@@ -226,9 +245,10 @@ describe('MatcherPatternPathDynamic', () => {
     const pattern = new MatcherPatternPathDynamic(
       /^\/teams\/(.*)$/i,
       {
-        pathMatch: {},
+        pathMatch: [{}],
       },
-      ['teams', 0]
+      ['teams', 0],
+      null
     )
     expect(pattern.match('/teams/')).toEqual({ pathMatch: '' })
     expect(pattern.match('/teams/123/b')).toEqual({ pathMatch: '123/b' })
@@ -245,7 +265,7 @@ describe('MatcherPatternPathDynamic', () => {
     const pattern = new MatcherPatternPathDynamic(
       /^\/teams(?:\/(.+?))?\/b$/i,
       {
-        teamId: { repeat: true },
+        teamId: [{}, true, true],
       },
       ['teams', 1, 'b']
     )
@@ -268,8 +288,8 @@ describe('MatcherPatternPathDynamic', () => {
     const pattern = new MatcherPatternPathDynamic(
       /^\/teams\/([^/]+?)\/([^/]+?)$/i,
       {
-        teamId: {},
-        otherId: {},
+        teamId: [{}],
+        otherId: [{}],
       },
       ['teams', 1, 1]
     )
@@ -290,8 +310,8 @@ describe('MatcherPatternPathDynamic', () => {
     const pattern = new MatcherPatternPathDynamic(
       /^\/teams\/([^/]+?)-b-([^/]+?)$/i,
       {
-        teamId: {},
-        otherId: {},
+        teamId: [{}],
+        otherId: [{}],
       },
       ['teams', [1, '-b-', 1]]
     )
@@ -312,9 +332,10 @@ describe('MatcherPatternPathDynamic', () => {
     const pattern = new MatcherPatternPathDynamic(
       /^\/teams\/([^/]+?)\/$/i,
       {
-        teamId: {},
+        teamId: [{}],
       },
-      ['teams', [1, '/']]
+      ['teams', 1],
+      true
     )
 
     expect(pattern.match('/teams/123/')).toEqual({
@@ -326,10 +347,11 @@ describe('MatcherPatternPathDynamic', () => {
     expect(pattern.build({ teamId: '123' })).toBe('/teams/123/')
   })
 
-  it('can have a trailing slash after a static segment', () => {
+  it.todo('can have a trailing slash after a static segment', () => {
     const pattern = new MatcherPatternPathDynamic(/^\/teams\/b\/$/i, {}, [
       'teams',
-      ['b', '/'],
+      'b',
+      ['/'],
     ])
 
     expect(pattern.match('/teams/b/')).toEqual({})
@@ -343,9 +365,10 @@ describe('MatcherPatternPathDynamic', () => {
     const pattern = new MatcherPatternPathDynamic(
       /^\/teams\/(.+?)\/$/,
       {
-        teamId: { repeat: true },
+        teamId: [{}, true],
       },
-      ['teams', [1, '/']]
+      ['teams', 1],
+      true
     )
 
     expect(pattern.match('/teams/123/')).toEqual({ teamId: ['123'] })
@@ -359,13 +382,14 @@ describe('MatcherPatternPathDynamic', () => {
     expect(pattern.build({ teamId: ['123', '456'] })).toBe('/teams/123/456/')
   })
 
-  it.todo('can have a trailing slash after optional repeatable param', () => {
+  it('can have a trailing slash after optional repeatable param', () => {
     const pattern = new MatcherPatternPathDynamic(
       /^\/teams(?:\/(.+?))?\/$/,
       {
-        teamId: { repeat: true },
+        teamId: [{}, true, true],
       },
-      ['teams', [1, '/']]
+      ['teams', 1],
+      true
     )
 
     expect(pattern.match('/teams/123/')).toEqual({ teamId: ['123'] })

packages/router/src/experimental/route-resolver/matchers/matcher-pattern.test-d.ts

@@ -8,7 +8,7 @@ describe('MatcherPatternPathDynamic', () => {
   it('can be generic', () => {
     const matcher = new MatcherPatternPathDynamic(
       /^\/users\/([^/]+)$/i,
-      { userId: { ...PATH_PARAM_PARSER_DEFAULTS } },
+      { userId: [PATH_PARAM_PARSER_DEFAULTS] },
       ['users', 1]
     )
 
@@ -33,7 +33,7 @@ describe('MatcherPatternPathDynamic', () => {
   it('can be a simple param', () => {
     const matcher = new MatcherPatternPathDynamic(
       /^\/users\/([^/]+)\/([^/]+)$/i,
-      { userId: { ...PATH_PARAM_SINGLE_DEFAULT, repeat: true } },
+      { userId: [PATH_PARAM_SINGLE_DEFAULT, true] },
       ['users', 1]
     )
     expectTypeOf(matcher.match('/users/123/456')).toEqualTypeOf<{
@@ -52,10 +52,10 @@ describe('MatcherPatternPathDynamic', () => {
     const matcher = new MatcherPatternPathDynamic(
       /^\/profiles\/([^/]+)$/i,
       {
-        userId: {
-          ...PARAM_INTEGER_SINGLE,
+        userId: [
+          PARAM_INTEGER_SINGLE,
           // parser: PATH_PARAM_DEFAULT_PARSER,
-        },
+        ],
       },
       ['profiles', 1]
     )

packages/router/src/experimental/route-resolver/matchers/matcher-pattern.ts

@@ -88,19 +88,25 @@ export class MatcherPatternPathStatic
 /**
  * Options for param parsers in {@link MatcherPatternPathDynamic}.
  */
-export interface MatcherPatternPathDynamic_ParamOptions<
+export type MatcherPatternPathDynamic_ParamOptions<
   TIn extends string | string[] | null = string | string[] | null,
   TOut = string | string[] | null,
-> extends ParamParser<TOut, TIn> {
+> = [
+  /**
+   * Param parser to use for this param.
+   */
+  parser: ParamParser<TOut, TIn>,
+
   /**
    * Is tha param a repeatable param and should be converted to an array
    */
-  repeat?: boolean
+  repeatable?: boolean,
 
-  // NOTE: not needed because in the regexp, the value is undefined if
-  // the group is optional and not given
-  // optional?: boolean
-}
+  /**
+   * Can this parameter be omitted or empty (for repeatable params, an empty array).
+   */
+  optional?: boolean,
+]
 
 /**
  * Helper type to extract the params from the options object.
@@ -115,6 +121,13 @@ type ExtractParamTypeFromOptions<TParamsOptions> = {
     : never
 }
 
+/**
+ * Regex to remove trailing slashes from a path.
+ *
+ * @internal
+ */
+const RE_TRAILING_SLASHES = /\/*$/
+
 /**
  * Handles the `path` part of a URL with dynamic parameters.
  */
@@ -138,12 +151,21 @@ export class MatcherPatternPathDynamic<
     readonly params: TParamsOptions &
       Record<string, MatcherPatternPathDynamic_ParamOptions<any, any>>,
     // 1 means a regular param, 0 means a splat, the order comes from the keys in params
-    readonly pathParts: Array<string | number | Array<string | number>>
+    readonly pathParts: Array<string | number | Array<string | number>>,
+    // null means "do not care", it's only for splat params
+    readonly trailingSlash: boolean | null = false
   ) {
     this.paramsKeys = Object.keys(this.params) as Array<keyof TParamsOptions>
   }
 
   match(path: string): ExtractParamTypeFromOptions<TParamsOptions> {
+    if (
+      this.trailingSlash != null &&
+      this.trailingSlash === !path.endsWith('/')
+    ) {
+      throw miss()
+    }
+
     const match = path.match(this.re)
     if (!match) {
       throw miss()
@@ -152,14 +174,14 @@ export class MatcherPatternPathDynamic<
     for (var i = 0; i < this.paramsKeys.length; i++) {
       // var for performance in for loop
       var paramName = this.paramsKeys[i]
-      var paramOptions = this.params[paramName]
+      var [parser, repeatable] = this.params[paramName]
       var currentMatch = (match[i + 1] as string | undefined) ?? null
 
-      var value = paramOptions.repeat
+      var value = repeatable
         ? (currentMatch?.split('/') || []).map<string>(decode)
         : decode(currentMatch)
 
-      params[paramName] = (paramOptions.get || identityFn)(value)
+      params[paramName] = (parser.get || identityFn)(value)
     }
 
     if (
@@ -177,11 +199,13 @@ export class MatcherPatternPathDynamic<
   build(params: ExtractParamTypeFromOptions<TParamsOptions>): string {
     let paramIndex = 0
     let paramName: keyof TParamsOptions
-    let paramOptions: (TParamsOptions &
+    let parser: (TParamsOptions &
       Record<
         string,
         MatcherPatternPathDynamic_ParamOptions<any, any>
-      >)[keyof TParamsOptions]
+      >)[keyof TParamsOptions][0]
+    let repeatable: boolean | undefined
+    let optional: boolean | undefined
     let lastParamPart: number | undefined
     let value: ReturnType<NonNullable<ParamParser['set']>> | undefined
     const path =
@@ -192,9 +216,13 @@ export class MatcherPatternPathDynamic<
             return part
           } else if (typeof part === 'number') {
             paramName = this.paramsKeys[paramIndex++]
-            paramOptions = this.params[paramName]
+            ;[parser, repeatable, optional] = this.params[paramName]
             lastParamPart = part
-            value = (paramOptions.set || identityFn)(params[paramName])
+            value = (parser.set || identityFn)(params[paramName])
+
+            if (Array.isArray(value) && !value.length && !optional) {
+              throw miss()
+            }
 
             return Array.isArray(value)
               ? value.map(encodeParam).join('/')
@@ -208,11 +236,11 @@ export class MatcherPatternPathDynamic<
                 }
 
                 paramName = this.paramsKeys[paramIndex++]
-                paramOptions = this.params[paramName]
-                value = (paramOptions.set || identityFn)(params[paramName])
+                ;[parser, repeatable, optional] = this.params[paramName]
+                value = (parser.set || identityFn)(params[paramName])
 
                 // param cannot be repeatable when in a sub segment
-                if (__DEV__ && paramOptions.repeat) {
+                if (__DEV__ && repeatable) {
                   warn(
                     `Param "${String(paramName)}" is repeatable, but used in a sub segment of the path: "${this.pathParts.join('')}". Repeated params can only be used as a full path segment: "/file/[ids]+/something-else". This will break in production.`
                   )
@@ -235,9 +263,9 @@ export class MatcherPatternPathDynamic<
      * with the original splat path: e.g. /teams/[...pathMatch] does not match /teams, so it makes
      * no sense to build a path it cannot match.
      */
-    return !lastParamPart /** lastParamPart == 0 */ && !value
-      ? path + '/'
-      : path
+    return this.trailingSlash == null
+      ? path + (!value ? '/' : '')
+      : path.replace(RE_TRAILING_SLASHES, this.trailingSlash ? '/' : '')
   }
 }