Fix #629 - add warning when named route has a default child route. (#641)

* fix  #629 - add warning when named route has a default child route.

* Improve warning message.

* this fails me

* debug log

* corrected Object.keys() mistake, shrunk test case to one case, still fails

* only warn when not in production mode.

* remove the helper, solved it with a normal jasmine spy

* ad child component again

* remove msg variable, check with `toMatch()` instead.

* remove unnecessary test case.

* remove unnecessary const, create it in beforeAll instead. change testacy to revert to this.maps instead of maps

* add test case for console.warn spy

* remove debug log

* add replace plugin for rollup to define "globals" like with web packs DefinePlugin.

* add replace plugin to rollup config and set production env variable in pm build script.

* Importe test case to test warning in production and development mode.

* fix unrelated lining bug: debugger statement in a file.

* cleanup unnecessary statement from test case

Author: LinusBorg

build/rollup.config.js

@@ -2,14 +2,19 @@ const buble = require('rollup-plugin-buble')
 const flow = require('rollup-plugin-flow')
 const cjs = require('rollup-plugin-commonjs')
 const node = require('rollup-plugin-node-resolve')
+const replace = require('rollup-plugin-replace')
 const version = process.env.VERSION || require('../package.json').version
 
+const replaceConfig = {
+  'process.env.NODE_ENV': JSON.stringify(process.env.NODE_ENV || 'development')
+}
+
 module.exports = {
   entry: 'src/index.js',
   dest: 'dist/vue-router.js',
   format: 'umd',
   moduleName: 'VueRouter',
-  plugins: [flow(), node(), cjs(), buble()],
+  plugins: [flow(), replace(replaceConfig), node(), cjs(), buble()],
   banner:
 `/**
  * vue-router v${version}

package.json

@@ -18,7 +18,7 @@
   "scripts": {
     "dev": "node examples/server.js",
     "dev:dist": "rollup -wm -c build/rollup.config.js",
-    "build": "rollup -c build/rollup.config.js && uglifyjs dist/vue-router.js -cm --comments -o dist/vue-router.min.js",
+    "build": "rollup --environment NODE_ENV:production -c build/rollup.config.js && uglifyjs dist/vue-router.js -cm --comments -o dist/vue-router.min.js",
     "lint": "eslint src examples",
     "test": "npm run lint && flow check && npm run test:unit && npm run test:e2e",
     "test:unit": "jasmine JASMINE_CONFIG_PATH=test/unit/jasmine.json",
@@ -53,6 +53,7 @@
     "rollup-plugin-commonjs": "^3.1.0",
     "rollup-plugin-flow": "^1.0.0",
     "rollup-plugin-node-resolve": "^2.0.0",
+    "rollup-plugin-replace": "^1.1.1",
     "rollup-watch": "^2.4.0",
     "selenium-server": "^2.53.1",
     "uglify-js": "^2.7.0",

src/create-route-map.js

@@ -1,6 +1,6 @@
 /* @flow */
 
-import { assert } from './util/warn'
+import { assert, warn } from './util/warn'
 import { cleanPath } from './util/path'
 
 export function createRouteMap (routes: Array<RouteConfig>): {
@@ -43,6 +43,17 @@ function addRouteRecord (
   }
 
   if (route.children) {
+    // Warn if route is named and has a default child route.
+    // If users navigate to this route by name, the default child will
+    // not be rendered (GH Issue #629)
+    if (process.env.NODE_ENV !== 'production') {
+      if (route.name && route.children.some(child => /^\/?$/.test(child.path))) {
+        warn(false, `Named Route '${route.name}' has a default child route.
+          When navigating to this named route (:to="{name: '${route.name}'"), the default child route will not be rendered.
+          Remove the name from this route and use the name of the default child route for named links instead.`
+        )
+      }
+    }
     route.children.forEach(child => {
       addRouteRecord(pathMap, nameMap, child, record)
     })

src/history/abstract.js

@@ -28,7 +28,6 @@ export class AbstractHistory extends History {
 
   go (n: number) {
     const targetIndex = this.index + n
-    if (!this.stack) debugger
     if (targetIndex < 0 || targetIndex >= this.stack.length) {
       return
     }

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

@@ -0,0 +1,54 @@
+/*eslint-disable no-undef*/
+import { createRouteMap } from '../../../src/create-route-map'
+
+const Home = { template: '<div>This is Home</div>' }
+const Foo = { template: '<div>This is Foo</div>' }
+const Bar = { template: '<div>This is Bar <router-view></router-view></div>' }
+const Baz = { template: '<div>This is Baz</div>' }
+
+const routes = [
+  { path: '/', name: 'home', component: Home },
+  { path: '/foo', name: 'foo', component: Foo },
+  {
+    path: '/bar',
+    name: 'bar',
+    component: Bar,
+    children: [
+      {
+        path: '',
+        component: Baz,
+        name: 'bar.baz'
+      }
+    ]
+  }
+]
+
+describe('Creating Route Map', function () {
+  beforeAll(function () {
+    spyOn(console, 'warn')
+    this.maps = createRouteMap(routes)
+  })
+  beforeEach(function () {
+    console.warn.calls.reset()
+    process.env.NODE_ENV = 'production'
+  })
+
+  it('has a pathMap object for default subroute at /bar/', function () {
+    expect(this.maps.pathMap['/bar/']).not.toBeUndefined()
+  })
+
+  it('has a nameMap object for default subroute at \'bar.baz\'', function () {
+    expect(this.maps.nameMap['bar.baz']).not.toBeUndefined()
+  })
+
+  it('in development, has logged a warning concering named route of parent and default subroute', function () {
+    process.env.NODE_ENV = 'development'
+    this.maps = createRouteMap(routes)
+    expect(console.warn).toHaveBeenCalled()
+    expect(console.warn.calls.argsFor(0)[0]).toMatch('vue-router] Named Route \'bar\'')
+  })
+  it('in production, it has not logged this warning', function () {
+    this.maps = createRouteMap(routes)
+    expect(console.warn).not.toHaveBeenCalled()
+  })
+})