From d79ea43367223cfe75b7f73c8c4453ec62b321bd Mon Sep 17 00:00:00 2001 From: Jimmy Jia Date: Mon, 4 Apr 2016 22:57:36 -0400 Subject: [PATCH] Deprecate routes with nested absolute paths --- modules/__tests__/isActive-test.js | 9 ++++ modules/__tests__/matchRoutes-test.js | 28 +++++++----- modules/isActive.js | 2 + modules/matchRoutes.js | 63 ++++++++++++++++----------- 4 files changed, 65 insertions(+), 37 deletions(-) diff --git a/modules/__tests__/isActive-test.js b/modules/__tests__/isActive-test.js index d938ebe4b0..bc0876b85a 100644 --- a/modules/__tests__/isActive-test.js +++ b/modules/__tests__/isActive-test.js @@ -6,6 +6,7 @@ import IndexRoute from '../IndexRoute' import Router from '../Router' import Route from '../Route' import qs from 'qs' +import shouldWarn from './shouldWarn' describe('isActive', function () { @@ -189,6 +190,10 @@ describe('isActive', function () { }) describe('a pathname that matches a parent route, but not the URL directly', function () { + beforeEach(() => { + shouldWarn('deprecated') + }) + describe('with no query', function () { it('is active', function (done) { render(( @@ -251,6 +256,10 @@ describe('isActive', function () { }) describe('a pathname that matches a nested absolute path', function () { + beforeEach(() => { + shouldWarn('deprecated') + }) + describe('with no query', function () { it('is active', function (done) { render(( diff --git a/modules/__tests__/matchRoutes-test.js b/modules/__tests__/matchRoutes-test.js index 23e97ef123..b985d2ec7b 100644 --- a/modules/__tests__/matchRoutes-test.js +++ b/modules/__tests__/matchRoutes-test.js @@ -11,8 +11,7 @@ describe('matchRoutes', function () { let routes let RootRoute, UsersRoute, UsersIndexRoute, UserRoute, PostRoute, FilesRoute, - AboutRoute, TeamRoute, ProfileRoute, GreedyRoute, OptionalRoute, - OptionalRouteChild, CatchAllRoute + AboutRoute, GreedyRoute, OptionalRoute, OptionalRouteChild, CatchAllRoute let createLocation = createMemoryHistory().createLocation beforeEach(function () { @@ -20,10 +19,7 @@ describe('matchRoutes', function () { - - - - + @@ -42,16 +38,10 @@ describe('matchRoutes', function () { UserRoute = { path: ':userID', childRoutes: [ - ProfileRoute = { - path: '/profile' - }, PostRoute = { path: ':postID' } ] - }, - TeamRoute = { - path: '/team' } ] } @@ -234,6 +224,13 @@ describe('matchRoutes', function () { describe('when the location matches a nested absolute route', function () { it('matches the correct routes', function (done) { + shouldWarn('deprecated') + + const TeamRoute = { + path: '/team' + } + UsersRoute.childRoutes.push(TeamRoute) + matchRoutes(routes, createLocation('/team'), function (error, match) { expect(match).toExist() expect(match.routes).toEqual([ RootRoute, UsersRoute, TeamRoute ]) @@ -244,6 +241,13 @@ describe('matchRoutes', function () { describe('when the location matches an absolute route nested under a route with params', function () { it('matches the correct routes and params', function (done) { + shouldWarn('deprecated') + + const ProfileRoute = { + path: '/profile' + } + UserRoute.childRoutes.push(ProfileRoute) + matchRoutes(routes, createLocation('/profile'), function (error, match) { expect(match).toExist() expect(match.routes).toEqual([ RootRoute, UsersRoute, UserRoute, ProfileRoute ]) diff --git a/modules/isActive.js b/modules/isActive.js index 54745cd6c5..7e8bc8d492 100644 --- a/modules/isActive.js +++ b/modules/isActive.js @@ -53,6 +53,8 @@ function getMatchingRouteIndex(pathname, activeRoutes, activeParams) { const pattern = route.path || '' if (pattern.charAt(0) === '/') { + // This code path is deprecated, but we the deprecation warning will + // actually be hit from matchRoutes, not from here. remainingPathname = pathname paramNames = [] paramValues = [] diff --git a/modules/matchRoutes.js b/modules/matchRoutes.js index 3373d53f32..f03000bbed 100644 --- a/modules/matchRoutes.js +++ b/modules/matchRoutes.js @@ -80,6 +80,13 @@ function matchRouteDeep( let pattern = route.path || '' if (pattern.charAt(0) === '/') { + if (remainingPathname !== location.pathname) { + warning( + false, + 'Nested routes with absolute paths are deprecated. Nest those routes under pathless routes instead. http://tiny.cc/router-decouple-ui' + ) + } + remainingPathname = location.pathname paramNames = [] paramValues = [] @@ -91,34 +98,40 @@ function matchRouteDeep( paramNames = [ ...paramNames, ...matched.paramNames ] paramValues = [ ...paramValues, ...matched.paramValues ] - if (remainingPathname === '' && route.path) { - const match = { - routes: [ route ], - params: createParams(paramNames, paramValues) - } + if (remainingPathname === '') { + if (route.path) { + const match = { + routes: [ route ], + params: createParams(paramNames, paramValues) + } - getIndexRoute(route, location, function (error, indexRoute) { - if (error) { - callback(error) - } else { - if (Array.isArray(indexRoute)) { - warning( - indexRoute.every(route => !route.path), - 'Index routes should not have paths' - ) - match.routes.push(...indexRoute) - } else if (indexRoute) { - warning( - !indexRoute.path, - 'Index routes should not have paths' - ) - match.routes.push(indexRoute) + getIndexRoute(route, location, function (error, indexRoute) { + if (error) { + callback(error) + } else { + if (Array.isArray(indexRoute)) { + warning( + indexRoute.every(route => !route.path), + 'Index routes should not have paths' + ) + match.routes.push(...indexRoute) + } else if (indexRoute) { + warning( + !indexRoute.path, + 'Index routes should not have paths' + ) + match.routes.push(indexRoute) + } + + callback(null, match) } + }) - callback(null, match) - } - }) - return + return + } + + // Re-normalize remaining path for continued match. + remainingPathname = '/' } }