Skip to content

Commit

Permalink
fix: removes warning resolving asterisk routes
Browse files Browse the repository at this point in the history
* fix: removes warning resolving asterisk routes

Closes #2505

* refactor: add test for #2505

* test: fix bad route name in test

* fix(flow): enuser params is an object
  • Loading branch information
posva authored Dec 23, 2018
1 parent 32ad6da commit e224637
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 3 deletions.
10 changes: 9 additions & 1 deletion src/util/params.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,23 @@ export function fillParams (
params: ?Object,
routeMsg: string
): string {
params = params || {}
try {
const filler =
regexpCompileCache[path] ||
(regexpCompileCache[path] = Regexp.compile(path))
return filler(params || {}, { pretty: true })

// 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 {
// delete the 0 if it was added
delete params[0]
}
}
41 changes: 39 additions & 2 deletions test/unit/specs/create-matcher.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ 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: '/error/*', name: 'error', component: { name: 'error' }},
{ path: '*', props: true, name: 'notFound', component: { name: 'notFound' }}
]

describe('Creating Matcher', function () {
Expand All @@ -26,14 +28,49 @@ 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 () {
match({ name: 'foo' }, routes[0])
expect(console.warn).not.toHaveBeenCalled()
})

it('matches named route with params without warning', function () {
process.env.NODE_ENV = 'development'
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]
)
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' })
Expand Down

1 comment on commit e224637

@adi-zz
Copy link
Contributor

@adi-zz adi-zz commented on e224637 Jan 11, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any plans on a new release that would include this fix?

Please sign in to comment.