From: Eduardo San Martin Morote <posva13@gmail.com>
Date: Thu, 6 Aug 2020 13:19:48 +0000 (+0200)
Subject: fix(matcher): avoid trailing slash with optional params
X-Git-Tag: v4.0.0-beta.7~20
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=faf0aab6451848e5b4330e1d01033137a0c42a5a;p=thirdparty%2Fvuejs%2Frouter.git

fix(matcher): avoid trailing slash with optional params
---

diff --git a/__tests__/matcher/pathParser.spec.ts b/__tests__/matcher/pathParser.spec.ts
index 2524d0ee..c3355ea6 100644
--- 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 5b9495cc..1a5b1ede 100644
--- 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 1bb7b07a..268e285e 100644
--- 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 663f2bf2..fad25118 100644
--- 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
         }