From bbfb889fad37ed3de26ecf2c5962ac6a146b7a95 Mon Sep 17 00:00:00 2001 From: "Xin Du (Clark)" Date: Sat, 22 Dec 2018 19:19:06 +0000 Subject: [PATCH 1/4] fix: removes warning resolving asterisk routes Closes #2505 --- src/util/params.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/util/params.js b/src/util/params.js index 4daf13cec..f2c1e2c6b 100644 --- a/src/util/params.js +++ b/src/util/params.js @@ -17,11 +17,18 @@ export function fillParams ( const filler = regexpCompileCache[path] || (regexpCompileCache[path] = Regexp.compile(path)) + if (params && params.pathMatch) { + params[0] = params.pathMatch + } return filler(params || {}, { pretty: true }) } catch (e) { if (process.env.NODE_ENV !== 'production') { warn(false, `missing param for ${routeMsg}: ${e.message}`) } return '' + } finally { + if (params && params[0]) { + delete params[0] + } } } From 67b46a245928d4a9c187c0af04b082b5cf873504 Mon Sep 17 00:00:00 2001 From: Eduardo San Martin Morote Date: Sat, 22 Dec 2018 20:27:19 +0100 Subject: [PATCH 2/4] refactor: add test for #2505 --- src/util/params.js | 15 ++++++++------- test/unit/specs/create-matcher.spec.js | 20 ++++++++++++++++++-- 2 files changed, 26 insertions(+), 9 deletions(-) diff --git a/src/util/params.js b/src/util/params.js index f2c1e2c6b..4f3d0fa11 100644 --- a/src/util/params.js +++ b/src/util/params.js @@ -17,18 +17,19 @@ export function fillParams ( const filler = regexpCompileCache[path] || (regexpCompileCache[path] = Regexp.compile(path)) - if (params && params.pathMatch) { - params[0] = params.pathMatch - } - return filler(params || {}, { pretty: true }) + + params = params || {} + // Fix #2505 resolving asterisk routes { name: 'not-found', params: { pathMatch: '/not-found' }} + if (params.pathMatch) params[0] = params.pathMatch + + return filler(params, { pretty: true }) } catch (e) { if (process.env.NODE_ENV !== 'production') { warn(false, `missing param for ${routeMsg}: ${e.message}`) } return '' } finally { - if (params && params[0]) { - delete params[0] - } + // delete the 0 if it was added + delete params[0] } } diff --git a/test/unit/specs/create-matcher.spec.js b/test/unit/specs/create-matcher.spec.js index 5028957e5..09aa0711f 100644 --- a/test/unit/specs/create-matcher.spec.js +++ b/test/unit/specs/create-matcher.spec.js @@ -4,7 +4,8 @@ import { createMatcher } from '../../../src/create-matcher' const routes = [ { path: '/', name: 'home', component: { name: 'home' }}, { path: '/foo', name: 'foo', component: { name: 'foo' }}, - { path: '*', props: true, component: { name: 'notFound' }} + { path: '/baz/:testparam', name: 'baz', component: { name: 'baz' }}, + { path: '*', props: true, name: 'notFound', component: { name: 'notFound' }} ] describe('Creating Matcher', function () { @@ -26,7 +27,7 @@ describe('Creating Matcher', function () { expect(matched.length).toBe(0) expect(name).toBe('bar') expect(console.warn).toHaveBeenCalled() - expect(console.warn.calls.argsFor(0)[0]).toMatch('Route with name \'bar\' does not exist') + expect(console.warn.calls.argsFor(0)[0]).toMatch("Route with name 'bar' does not exist") }) it('in production, it has not logged this warning', function () { @@ -34,6 +35,21 @@ describe('Creating Matcher', function () { expect(console.warn).not.toHaveBeenCalled() }) + it('matches named route with params without warning', function () { + process.env.NODE_ENV = 'development' + const { name, path } = match({ name: 'baz', params: { testparam: 'testvalue' }}) + expect(console.warn).not.toHaveBeenCalled() + expect(name).toEqual('baz') + expect(path).toEqual('/baz/testvalue') + }) + + it('matches asterisk routes with a default param name without warning', function () { + process.env.NODE_ENV = 'development' + const { params } = match({ name: 'notfound', params: { pathMatch: '/not-found' }}, routes[0]) + expect(console.warn).not.toHaveBeenCalled() + expect(params).toEqual({ pathMatch: '/not-found' }) + }) + it('matches asterisk routes with a default param name', function () { const { params } = match({ path: '/not-found' }, routes[0]) expect(params).toEqual({ pathMatch: '/not-found' }) From 8a8ffcf5b1c8ab8b595a6ab29f9db98e74058c96 Mon Sep 17 00:00:00 2001 From: Eduardo San Martin Morote Date: Sun, 23 Dec 2018 13:41:52 +0100 Subject: [PATCH 3/4] test: fix bad route name in test --- test/unit/specs/create-matcher.spec.js | 27 +++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/test/unit/specs/create-matcher.spec.js b/test/unit/specs/create-matcher.spec.js index 09aa0711f..88fb475a1 100644 --- a/test/unit/specs/create-matcher.spec.js +++ b/test/unit/specs/create-matcher.spec.js @@ -5,6 +5,7 @@ const routes = [ { path: '/', name: 'home', component: { name: 'home' }}, { path: '/foo', name: 'foo', component: { name: 'foo' }}, { path: '/baz/:testparam', name: 'baz', component: { name: 'baz' }}, + { path: '/error/*', name: 'error', component: { name: 'error' }}, { path: '*', props: true, name: 'notFound', component: { name: 'notFound' }} ] @@ -27,7 +28,9 @@ describe('Creating Matcher', function () { expect(matched.length).toBe(0) expect(name).toBe('bar') expect(console.warn).toHaveBeenCalled() - expect(console.warn.calls.argsFor(0)[0]).toMatch("Route with name 'bar' does not exist") + expect(console.warn.calls.argsFor(0)[0]).toMatch( + "Route with name 'bar' does not exist" + ) }) it('in production, it has not logged this warning', function () { @@ -37,19 +40,37 @@ describe('Creating Matcher', function () { it('matches named route with params without warning', function () { process.env.NODE_ENV = 'development' - const { name, path } = match({ name: 'baz', params: { testparam: 'testvalue' }}) + const { name, path, params } = match({ + name: 'baz', + params: { testparam: 'testvalue' } + }) expect(console.warn).not.toHaveBeenCalled() expect(name).toEqual('baz') expect(path).toEqual('/baz/testvalue') + expect(params).toEqual({ testparam: 'testvalue' }) }) it('matches asterisk routes with a default param name without warning', function () { process.env.NODE_ENV = 'development' - const { params } = match({ name: 'notfound', params: { pathMatch: '/not-found' }}, routes[0]) + const { params } = match( + { name: 'notFound', params: { pathMatch: '/not-found' }}, + routes[0] + ) expect(console.warn).not.toHaveBeenCalled() expect(params).toEqual({ pathMatch: '/not-found' }) }) + it('matches partial asterisk routes with a default param name without warning', function () { + process.env.NODE_ENV = 'development' + const { params, path } = match( + { name: 'error', params: { pathMatch: 'some' }}, + routes[0] + ) + expect(console.warn).not.toHaveBeenCalled() + expect(params).toEqual({ pathMatch: 'some' }) + expect(path).toEqual('/error/some') + }) + it('matches asterisk routes with a default param name', function () { const { params } = match({ path: '/not-found' }, routes[0]) expect(params).toEqual({ pathMatch: '/not-found' }) From 34d4cac35c6dcce48ee94179adf1ca0ae092cb66 Mon Sep 17 00:00:00 2001 From: Eduardo San Martin Morote Date: Sun, 23 Dec 2018 13:45:35 +0100 Subject: [PATCH 4/4] fix(flow): enuser params is an object --- src/util/params.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/params.js b/src/util/params.js index 4f3d0fa11..cac6a683d 100644 --- a/src/util/params.js +++ b/src/util/params.js @@ -13,12 +13,12 @@ export function fillParams ( params: ?Object, routeMsg: string ): string { + params = params || {} try { const filler = regexpCompileCache[path] || (regexpCompileCache[path] = Regexp.compile(path)) - params = params || {} // Fix #2505 resolving asterisk routes { name: 'not-found', params: { pathMatch: '/not-found' }} if (params.pathMatch) params[0] = params.pathMatch