diff --git a/modules/TransitionUtils.js b/modules/TransitionUtils.js index ef73814a52..078c8aab6d 100644 --- a/modules/TransitionUtils.js +++ b/modules/TransitionUtils.js @@ -1,11 +1,12 @@ import { loopAsync } from './AsyncUtils' import warning from './routerWarning' -function createEnterHook(hook, route) { - return function (a, b, callback) { - hook.apply(route, arguments) +function createTransitionHook(hook, route, asyncArity) { + return function (...args) { + hook.apply(route, args) - if (hook.length < 3) { + if (hook.length < asyncArity) { + let callback = args[args.length - 1] // Assume hook executes synchronously and // automatically call the callback. callback() @@ -16,26 +17,22 @@ function createEnterHook(hook, route) { function getEnterHooks(routes) { return routes.reduce(function (hooks, route) { if (route.onEnter) - hooks.push(createEnterHook(route.onEnter, route)) + hooks.push(createTransitionHook(route.onEnter, route, 3)) return hooks }, []) } -/** - * Runs all onEnter hooks in the given array of routes in order - * with onEnter(nextState, replace, callback) and calls - * callback(error, redirectInfo) when finished. The first hook - * to use replace short-circuits the loop. - * - * If a hook needs to run asynchronously, it may use the callback - * function. However, doing so will cause the transition to pause, - * which could lead to a non-responsive UI if the hook is slow. - */ -export function runEnterHooks(routes, nextState, callback) { - const hooks = getEnterHooks(routes) +function getChangeHooks(routes) { + return routes.reduce(function (hooks, route) { + if (route.onChange) + hooks.push(createTransitionHook(route.onChange, route, 4)) + return hooks + }, []) +} - if (!hooks.length) { +function runTransitionHooks(length, iter, callback) { + if (!length) { callback() return } @@ -59,8 +56,8 @@ export function runEnterHooks(routes, nextState, callback) { redirectInfo = location } - loopAsync(hooks.length, function (index, next, done) { - hooks[index](nextState, replace, function (error) { + loopAsync(length, function (index, next, done) { + iter(index, replace, function (error) { if (error || redirectInfo) { done(error, redirectInfo) // No need to continue. } else { @@ -70,6 +67,40 @@ export function runEnterHooks(routes, nextState, callback) { }, callback) } +/** + * Runs all onEnter hooks in the given array of routes in order + * with onEnter(nextState, replace, callback) and calls + * callback(error, redirectInfo) when finished. The first hook + * to use replace short-circuits the loop. + * + * If a hook needs to run asynchronously, it may use the callback + * function. However, doing so will cause the transition to pause, + * which could lead to a non-responsive UI if the hook is slow. + */ +export function runEnterHooks(routes, nextState, callback) { + const hooks = getEnterHooks(routes) + return runTransitionHooks(hooks.length, (index, replace, next) => { + hooks[index](nextState, replace, next) + }, callback) +} + +/** + * Runs all onChange hooks in the given array of routes in order + * with onChange(prevState, nextState, replace, callback) and calls + * callback(error, redirectInfo) when finished. The first hook + * to use replace short-circuits the loop. + * + * If a hook needs to run asynchronously, it may use the callback + * function. However, doing so will cause the transition to pause, + * which could lead to a non-responsive UI if the hook is slow. + */ +export function runChangeHooks(routes, state, nextState, callback) { + const hooks = getChangeHooks(routes) + return runTransitionHooks(hooks.length, (index, replace, next) => { + hooks[index](state, nextState, replace, next) + }, callback) +} + /** * Runs all onLeave hooks in the given array of routes in order. */ diff --git a/modules/__tests__/transitionHooks-test.js b/modules/__tests__/transitionHooks-test.js index fdff854334..2cbb4652f3 100644 --- a/modules/__tests__/transitionHooks-test.js +++ b/modules/__tests__/transitionHooks-test.js @@ -58,6 +58,13 @@ describe('When a router enters a branch', function () { expect(nextState.routes).toContain(NewsFeedRoute) expect(replace).toBeA('function') }, + onChange(prevState, nextState, replace) { + expect(this).toBe(NewsFeedRoute) + expect(prevState).toNotEqual(nextState) + expect(prevState.routes).toContain(NewsFeedRoute) + expect(nextState.routes).toContain(NewsFeedRoute) + expect(replace).toBeA('function') + }, onLeave() { expect(this).toBe(NewsFeedRoute) } @@ -97,6 +104,12 @@ describe('When a router enters a branch', function () { expect(nextState.routes).toContain(MessageRoute) expect(replace).toBeA('function') }, + onChange(prevState, nextState, replace) { + expect(this).toBe(MessageRoute) + expect(prevState.routes).toContain(MessageRoute) + expect(nextState.routes).toContain(MessageRoute) + expect(replace).toBeA('function') + }, onLeave() { expect(this).toBe(MessageRoute) } @@ -110,6 +123,13 @@ describe('When a router enters a branch', function () { expect(nextState.routes).toContain(DashboardRoute) expect(replace).toBeA('function') }, + onChange(prevState, nextState, replace) { + expect(this).toBe(DashboardRoute) + expect(prevState).toNotEqual(nextState) + expect(prevState.routes).toContain(DashboardRoute) + expect(nextState.routes).toContain(DashboardRoute) + expect(replace).toBeA('function') + }, onLeave() { expect(this).toBe(DashboardRoute) }, @@ -212,8 +232,11 @@ describe('When a router enters a branch', function () { describe('and then navigates to the same branch, but with different params', function () { it('calls the onLeave and onEnter hooks of all routes whose params have changed', function (done) { const dashboardRouteLeaveSpy = spyOn(DashboardRoute, 'onLeave').andCallThrough() + const dashboardRouteChangeSpy = spyOn(DashboardRoute, 'onChange').andCallThrough() const dashboardRouteEnterSpy = spyOn(DashboardRoute, 'onEnter').andCallThrough() + const messageRouteLeaveSpy = spyOn(MessageRoute, 'onLeave').andCallThrough() + const messageRouteChangeSpy = spyOn(MessageRoute, 'onChange').andCallThrough() const messageRouteEnterSpy = spyOn(MessageRoute, 'onEnter').andCallThrough() const history = createHistory('/messages/123') @@ -226,6 +249,9 @@ describe('When a router enters a branch', function () { function () { expect(messageRouteLeaveSpy).toHaveBeenCalled('MessageRoute.onLeave was not called') expect(messageRouteEnterSpy).toHaveBeenCalled('MessageRoute.onEnter was not called') + expect(messageRouteChangeSpy.calls.length).toEqual(0, 'DashboardRoute.onChange was called') + + expect(dashboardRouteChangeSpy).toHaveBeenCalled('DashboardRoute.onChange was not called') expect(dashboardRouteLeaveSpy.calls.length).toEqual(0, 'DashboardRoute.onLeave was called') } ] @@ -243,12 +269,16 @@ describe('When a router enters a branch', function () { describe('and then the query changes', function () { it('calls the onEnter hooks of all routes in that branch', function (done) { const newsFeedRouteEnterSpy = spyOn(NewsFeedRoute, 'onEnter').andCallThrough() + const newsFeedRouteChangeSpy = spyOn(NewsFeedRoute, 'onChange').andCallThrough() const history = useQueries(createHistory)('/inbox') render(, node, function () { history.push({ pathname: '/news', query: { q: 1 } }) + expect(newsFeedRouteChangeSpy.calls.length).toEqual(0, 'NewsFeedRoute.onChange was called') expect(newsFeedRouteEnterSpy.calls.length).toEqual(1) + history.push({ pathname: '/news', query: { q: 2 } }) + expect(newsFeedRouteChangeSpy).toHaveBeenCalled('NewsFeedRoute.onChange was not called') expect(newsFeedRouteEnterSpy.calls.length).toEqual(1) done() }) diff --git a/modules/computeChangedRoutes.js b/modules/computeChangedRoutes.js index 05c386dacc..c7152b008f 100644 --- a/modules/computeChangedRoutes.js +++ b/modules/computeChangedRoutes.js @@ -12,7 +12,7 @@ function routeParamsChanged(route, prevState, nextState) { } /** - * Returns an object of { leaveRoutes, enterRoutes } determined by + * Returns an object of { leaveRoutes, changeRoutes, enterRoutes } determined by * the change from prevState to nextState. We leave routes if either * 1) they are not in the next state or 2) they are in the next state * but their params have changed (i.e. /users/123 => /users/456). @@ -20,12 +20,15 @@ function routeParamsChanged(route, prevState, nextState) { * leaveRoutes are ordered starting at the leaf route of the tree * we're leaving up to the common parent route. enterRoutes are ordered * from the top of the tree we're entering down to the leaf route. + * + * changeRoutes are any routes that didn't leave or enter during + * the transition. */ function computeChangedRoutes(prevState, nextState) { const prevRoutes = prevState && prevState.routes const nextRoutes = nextState.routes - let leaveRoutes, enterRoutes + let leaveRoutes, changeRoutes, enterRoutes if (prevRoutes) { leaveRoutes = prevRoutes.filter(function (route) { return nextRoutes.indexOf(route) === -1 || routeParamsChanged(route, prevState, nextState) @@ -34,16 +37,28 @@ function computeChangedRoutes(prevState, nextState) { // onLeave hooks start at the leaf route. leaveRoutes.reverse() - enterRoutes = nextRoutes.filter(function (route) { - return prevRoutes.indexOf(route) === -1 || leaveRoutes.indexOf(route) !== -1 + enterRoutes = [] + changeRoutes = [] + + nextRoutes.forEach(function (route) { + const isNew = prevRoutes.indexOf(route) === -1 + const paramsChanged = leaveRoutes.indexOf(route) !== -1 + + if (isNew || paramsChanged) + enterRoutes.push(route) + else + changeRoutes.push(route) }) + } else { leaveRoutes = [] + changeRoutes = [] enterRoutes = nextRoutes } return { leaveRoutes, + changeRoutes, enterRoutes } } diff --git a/modules/createTransitionManager.js b/modules/createTransitionManager.js index 7bb5fe7726..bcf4639dc6 100644 --- a/modules/createTransitionManager.js +++ b/modules/createTransitionManager.js @@ -1,7 +1,7 @@ import warning from './routerWarning' import { REPLACE } from 'history/lib/Actions' import computeChangedRoutes from './computeChangedRoutes' -import { runEnterHooks, runLeaveHooks } from './TransitionUtils' +import { runEnterHooks, runChangeHooks, runLeaveHooks } from './TransitionUtils' import { default as _isActive } from './isActive' import getComponents from './getComponents' import matchRoutes from './matchRoutes' @@ -67,33 +67,43 @@ export default function createTransitionManager(history, routes) { } function finishMatch(nextState, callback) { - const { leaveRoutes, enterRoutes } = computeChangedRoutes(state, nextState) + const { leaveRoutes, changeRoutes, enterRoutes } = computeChangedRoutes(state, nextState) runLeaveHooks(leaveRoutes) // Tear down confirmation hooks for left routes leaveRoutes.forEach(removeListenBeforeHooksForRoute) - runEnterHooks(enterRoutes, nextState, function (error, redirectInfo) { - if (error) { - callback(error) - } else if (redirectInfo) { - callback(null, createLocationFromRedirectInfo(redirectInfo)) - } else { - // TODO: Fetch components after state is updated. - getComponents(nextState, function (error, components) { - if (error) { - callback(error) - } else { - // TODO: Make match a pure function and have some other API - // for "match and update state". - callback(null, null, ( - state = { ...nextState, components }) - ) - } - }) - } + // change and enter hooks are run in series + runChangeHooks(changeRoutes, state, nextState, (error, redirectInfo) => { + if (error || redirectInfo) + return handleErrorOrRedirect(error, redirectInfo) + + runEnterHooks(enterRoutes, nextState, finishEnterHooks) }) + + function finishEnterHooks(error, redirectInfo) { + if (error || redirectInfo) + return handleErrorOrRedirect(error, redirectInfo) + + // TODO: Fetch components after state is updated. + getComponents(nextState, function (error, components) { + if (error) { + callback(error) + } else { + // TODO: Make match a pure function and have some other API + // for "match and update state". + callback(null, null, ( + state = { ...nextState, components }) + ) + } + }) + } + + function handleErrorOrRedirect(error, redirectInfo) { + if (error) callback(error) + else callback(null, createLocationFromRedirectInfo(redirectInfo)) + } } let RouteGuid = 1