fix(matcher): avoid trailing slash with optional params

diff --git a/__tests__/matcher/pathParser.spec.ts b/__tests__/matcher/pathParser.spec.ts
index 2524d0ee8830b16dd8b25a8cf033ad38404fd460..c3355ea6119e720d8a66f8ed444b6a2678df8213 100644 (file)
--- a/__tests__/matcher/pathParser.spec.ts
+++ b/__tests__/matcher/pathParser.spec.ts
@@ -458,17 +458,21 @@ describe('Path parser', () => {
     })
 
     it('param?', () => {
-      matchRegExp('^(?:/(\\d+))?$', [
+      matchRegExp(
+        '^(?:/(\\d+))?/?$',
         [
-          {
-            type: TokenType.Param,
-            value: 'id',
-            regexp: '\\d+',
-            repeatable: false,
-            optional: true,
-          },
+          [
+            {
+              type: TokenType.Param,
+              value: 'id',
+              regexp: '\\d+',
+              repeatable: false,
+              optional: true,
+            },
+          ],
         ],
-      ])
+        { strict: false }
+      )
     })
 
     it('static and param?', () => {
@@ -651,6 +655,15 @@ describe('Path parser', () => {
       matchParams('/:a*', '/', { a: '' })
     })
 
+    it('static then empty param optional', () => {
+      matchParams('/a/:a?', '/a', { a: '' })
+      matchParams('/a/:a?', '/a/a', { a: 'a' })
+      matchParams('/a/:a?', '/a/a/', { a: 'a' })
+      matchParams('/a/:a?', '/a/', { a: '' })
+      matchParams('/a/:a*', '/a', { a: '' })
+      matchParams('/a/:a*', '/a/', { a: '' })
+    })
+
     it('static then param optional', () => {
       matchParams('/one/:a?', '/one/two', { a: 'two' })
       matchParams('/one/:a?', '/one/', { a: '' })
@@ -744,6 +757,11 @@ describe('Path parser', () => {
       matchStringify('/:a*', { a: ['one', 'two'] }, '/one/two')
     })
 
+    it('static then optional param?', () => {
+      matchStringify('/a/:a?', { a: '' }, '/a')
+      matchStringify('/a/:a?', {}, '/a')
+    })
+
     it('optional param?', () => {
       matchStringify('/:a?/other', { a: '' }, '/other')
       matchStringify('/:a?/other', {}, '/other')

diff --git a/__tests__/matcher/resolve.spec.ts b/__tests__/matcher/resolve.spec.ts
index 5b9495cc733a3479e079c1d914628ea0646a4042..1a5b1edea342fe18e753a4d34f7957d37113fad7 100644 (file)
--- a/__tests__/matcher/resolve.spec.ts
+++ b/__tests__/matcher/resolve.spec.ts
@@ -54,11 +54,8 @@ describe('RouterMatcher.resolve', () => {
     }
 
     // allows not passing params
-    if ('params' in location) {
-      resolved.params = resolved.params || location.params
-    } else {
-      resolved.params = resolved.params || {}
-    }
+    resolved.params =
+      resolved.params || ('params' in location ? location.params : {})
 
     const startCopy: MatcherLocation = {
       ...start,
@@ -670,6 +667,32 @@ describe('RouterMatcher.resolve', () => {
       )
     })
 
+    it('allows an optional trailing slash with optional param', () => {
+      assertRecordMatch(
+        { path: '/:a', components, name: 'a' },
+        { path: '/a/' },
+        { path: '/a/', params: { a: 'a' }, name: 'a' }
+      )
+      assertRecordMatch(
+        { path: '/a/:a', components, name: 'a' },
+        { path: '/a/a/' },
+        { path: '/a/a/', params: { a: 'a' }, name: 'a' }
+      )
+    })
+
+    it('allows an optional trailing slash with missing optional param', () => {
+      assertRecordMatch(
+        { path: '/:a?', components, name: 'a' },
+        { path: '/' },
+        { path: '/', params: { a: '' }, name: 'a' }
+      )
+      assertRecordMatch(
+        { path: '/a/:a?', components, name: 'a' },
+        { path: '/a/' },
+        { path: '/a/', params: { a: '' }, name: 'a' }
+      )
+    })
+
     // FIXME:
     it.skip('keeps required trailing slash (strict: true)', () => {
       const record = {
@@ -754,6 +777,21 @@ describe('RouterMatcher.resolve', () => {
         }
       )
     })
+
+    it('drops optional params', () => {
+      assertRecordMatch(
+        { path: '/:a/:b?', name: 'p', components },
+        { name: 'p', params: { a: 'b' } },
+        { name: 'p', path: '/b', params: { a: 'b' } },
+        {
+          params: { a: 'a', b: 'b' },
+          path: '/a',
+          matched: [],
+          meta: {},
+          name: undefined,
+        }
+      )
+    })
   })
 
   describe('LocationAsRelative', () => {
@@ -888,6 +926,36 @@ describe('RouterMatcher.resolve', () => {
       )
     })
 
+    it('keep optional params', () => {
+      assertRecordMatch(
+        { path: '/:a/:b?', name: 'p', components },
+        {},
+        { name: 'p', path: '/a/b', params: { a: 'a', b: 'b' } },
+        {
+          name: 'p',
+          params: { a: 'a', b: 'b' },
+          path: '/a/b',
+          matched: [],
+          meta: {},
+        }
+      )
+    })
+
+    it('merges optional params', () => {
+      assertRecordMatch(
+        { path: '/:a/:b?', name: 'p', components },
+        { params: { a: 'c' } },
+        { name: 'p', path: '/c/b', params: { a: 'c', b: 'b' } },
+        {
+          name: 'p',
+          params: { a: 'a', b: 'b' },
+          path: '/a/b',
+          matched: [],
+          meta: {},
+        }
+      )
+    })
+
     it('throws if the current named route does not exists', () => {
       const record = { path: '/', components }
       const start = {

diff --git a/src/matcher/index.ts b/src/matcher/index.ts
index 1bb7b07a865aba93c2ab2ec631008d81c86700df..268e285ebd2fb9eeab89d2e90cc34a1f46fc43fc 100644 (file)
--- a/src/matcher/index.ts
+++ b/src/matcher/index.ts
@@ -230,7 +230,7 @@ export function createRouterMatcher(
           currentLocation.params,
           // only keep params that exist in the resolved location
           // TODO: only keep optional params coming from a parent record
-          matcher.keys.map(k => k.name)
+          matcher.keys.filter(k => !k.optional).map(k => k.name)
         ),
         location.params
       )

diff --git a/src/matcher/pathParserRanker.ts b/src/matcher/pathParserRanker.ts
index 663f2bf257bf33d80687902957ce0c9af54cf44e..fad25118ed5482873774cc2321b26aed8c948938 100644 (file)
--- a/src/matcher/pathParserRanker.ts
+++ b/src/matcher/pathParserRanker.ts
@@ -218,7 +218,7 @@ export function tokensToParser(
     // for optional parameters to allow to be empty
     let avoidDuplicatedSlash: boolean = false
     for (const segment of segments) {
-      if (!avoidDuplicatedSlash || path[path.length - 1] !== '/') path += '/'
+      if (!avoidDuplicatedSlash || !path.endsWith('/')) path += '/'
       avoidDuplicatedSlash = false
 
       for (const token of segment) {
@@ -234,9 +234,12 @@ export function tokensToParser(
             )
           const text: string = Array.isArray(param) ? param.join('/') : param
           if (!text) {
-            // do not append a slash on the next iteration
-            if (optional) avoidDuplicatedSlash = true
-            else throw new Error(`Missing required param "${value}"`)
+            if (optional) {
+              // remove the last slash
+              if (path.endsWith('/')) path = path.slice(0, -1)
+              // do not append a slash on the next iteration
+              else avoidDuplicatedSlash = true
+            } else throw new Error(`Missing required param "${value}"`)
           }
           path += text
         }