refactor route regex compilation + warn duplicate param keys (close #1345)

Author: yyx990803

flow/declarations.js

@@ -1,8 +1,12 @@
 declare var document: Document;
 
+declare class RouteRegExp extends RegExp {
+  keys: Array<{ name: string, optional: boolean }>;
+}
+
 declare module 'path-to-regexp' {
   declare var exports: {
-    (path: string, keys: Array<?{ name: string }>): RegExp;
+    (path: string, keys?: Array<?{ name: string }>): RouteRegExp;
     compile: (path: string) => (params: Object) => string;
   }
 }
@@ -48,6 +52,7 @@ declare type RouteConfig = {
 
 declare type RouteRecord = {
   path: string;
+  regex: RouteRegExp;
   components: Dictionary<any>;
   instances: Dictionary<any>;
   name: ?string;

src/create-matcher.js

@@ -1,12 +1,12 @@
 /* @flow */
 
 import type VueRouter from './index'
+import { resolvePath } from './util/path'
 import { assert, warn } from './util/warn'
 import { createRoute } from './util/route'
+import { fillParams } from './util/params'
 import { createRouteMap } from './create-route-map'
-import { resolvePath } from './util/path'
 import { normalizeLocation } from './util/location'
-import { getRouteRegex, fillParams } from './util/params'
 
 export type Matcher = {
   match: (raw: RawLocation, current?: Route, redirectedFrom?: Location) => Route;
@@ -36,7 +36,7 @@ export function createMatcher (
       if (process.env.NODE_ENV !== 'production') {
         warn(record, `Route with name '${name}' does not exist`)
       }
-      const paramNames = getRouteRegex(record.path).keys
+      const paramNames = record.regex.keys
         .filter(key => !key.optional)
         .map(key => key.name)
 
@@ -60,8 +60,9 @@ export function createMatcher (
       location.params = {}
       for (let i = 0; i < pathList.length; i++) {
         const path = pathList[i]
-        if (matchRoute(path, location.params, location.path)) {
-          return _createRoute(pathMap[path], location, redirectedFrom)
+        const record = pathMap[path]
+        if (matchRoute(record.regex, location.path, location.params)) {
+          return _createRoute(record, location, redirectedFrom)
         }
       }
     }
@@ -171,12 +172,11 @@ export function createMatcher (
 }
 
 function matchRoute (
+  regex: RouteRegExp,
   path: string,
-  params: Object,
-  pathname: string
+  params: Object
 ): boolean {
-  const { regexp, keys } = getRouteRegex(path)
-  const m = pathname.match(regexp)
+  const m = path.match(regex)
 
   if (!m) {
     return false
@@ -185,9 +185,11 @@ function matchRoute (
   }
 
   for (let i = 1, len = m.length; i < len; ++i) {
-    const key = keys[i - 1]
+    const key = regex.keys[i - 1]
     const val = typeof m[i] === 'string' ? decodeURIComponent(m[i]) : m[i]
-    if (key) params[key.name] = val
+    if (key) {
+      params[key.name] = val
+    }
   }
 
   return true

src/create-route-map.js

@@ -1,7 +1,8 @@
 /* @flow */
 
-import { assert, warn } from './util/warn'
+import Regexp from 'path-to-regexp'
 import { cleanPath } from './util/path'
+import { assert, warn } from './util/warn'
 
 export function createRouteMap (
   routes: Array<RouteConfig>,
@@ -56,8 +57,10 @@ function addRouteRecord (
     )
   }
 
+  const normalizedPath = normalizePath(path, parent)
   const record: RouteRecord = {
-    path: normalizePath(path, parent),
+    path: normalizedPath,
+    regex: compileRouteRegex(normalizedPath),
     components: route.components || { default: route.component },
     instances: {},
     name,
@@ -133,6 +136,18 @@ function addRouteRecord (
   }
 }
 
+function compileRouteRegex (path: string): RouteRegExp {
+  const regex = Regexp(path)
+  if (process.env.NODE_ENV !== 'production') {
+    const keys: any = {}
+    regex.keys.forEach(key => {
+      warn(!keys[key], `Duplicate param keys in route with path: "${path}"`)
+      keys[key] = true
+    })
+  }
+  return regex
+}
+
 function normalizePath (path: string, parent?: RouteRecord): string {
   path = path.replace(/\/$/, '')
   if (path[0] === '/') return path

src/util/params.js

@@ -1,30 +1,7 @@
 /* @flow */
 
-import Regexp from 'path-to-regexp'
 import { warn } from './warn'
-
-const regexpCache: {
-  [key: string]: {
-    keys: Array<?{ name: string }>,
-    regexp: RegExp
-  }
-} = Object.create(null)
-
-export function getRouteRegex (path: string): Object {
-  const hit = regexpCache[path]
-  let keys, regexp
-
-  if (hit) {
-    keys = hit.keys
-    regexp = hit.regexp
-  } else {
-    keys = []
-    regexp = Regexp(path, keys)
-    regexpCache[path] = { keys, regexp }
-  }
-
-  return { keys, regexp }
-}
+import Regexp from 'path-to-regexp'
 
 const regexpCompileCache: {
   [key: string]: Function

test/unit/specs/create-map.spec.js

@@ -60,4 +60,18 @@ describe('Creating Route Map', function () {
     maps = createRouteMap(routes)
     expect(console.warn).not.toHaveBeenCalled()
   })
+
+  it('in development, warn duplicate param keys', () => {
+    process.env.NODE_ENV = 'development'
+    maps = createRouteMap([
+      {
+        path: '/foo/:id', component: Foo,
+        children: [
+          { path: 'bar/:id', component: Bar }
+        ]
+      }
+    ])
+    expect(console.warn).toHaveBeenCalled()
+    expect(console.warn.calls.argsFor(0)[0]).toMatch('vue-router] Duplicate param keys in route with path: "/foo/:id/bar/:id"')
+  })
 })

test/unit/specs/create-matcher.spec.js

@@ -23,7 +23,7 @@ describe('Creating Matcher', function () {
     process.env.NODE_ENV = 'development'
     expect(() => {
       match({ name: 'bar' }, routes[0]);
-    }).toThrow(new TypeError('Cannot read property \'path\' of undefined'));
+    }).toThrow(new TypeError('Cannot read property \'regex\' of undefined'));
     expect(console.warn).toHaveBeenCalled()
     expect(console.warn.calls.argsFor(0)[0]).toMatch('Route with name \'bar\' does not exist');
   })

types/router.d.ts

@@ -75,6 +75,7 @@ export interface RouteConfig {
 
 export interface RouteRecord {
   path: string;
+  regex: RegExp;
   components: Dictionary<Component>;
   instances: Dictionary<Vue>;
   name?: string;