Skip to content

Commit 90cd750

Browse files
committed
[fixed] Call all transition hooks on query changes
This commit fires transition hooks on all route handlers in the hierarchy whenever the query changes, not just the leaf route. This also fixes a regression in #499 that throws a TypeError when there are not yet any routes in the router state (see #623). Fixes #623
1 parent bd71b9c commit 90cd750

File tree

2 files changed

+36
-19
lines changed

2 files changed

+36
-19
lines changed

modules/__tests__/Router-test.js

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ var expect = require('expect');
22
var React = require('react');
33
var Router = require('../index');
44
var Route = require('../components/Route');
5+
var RouteHandler = require('../components/RouteHandler');
56
var TestLocation = require('../locations/TestLocation');
67
var ScrollToTopBehavior = require('../behaviors/ScrollToTopBehavior');
78
var getWindowScrollPosition = require('../utils/getWindowScrollPosition');
@@ -533,8 +534,22 @@ describe('Router', function () {
533534
});
534535
});
535536

536-
it('execute willTransition* callbacks when query changes', function (done) {
537-
var fromCallbackExecuted = false;
537+
it('executes transition hooks when only the query changes', function (done) {
538+
var fromKnifeCalled = false;
539+
var fromSpoonCalled = false;
540+
541+
var Knife = React.createClass({
542+
statics: {
543+
willTransitionFrom: function () {
544+
fromKnifeCalled = true;
545+
}
546+
},
547+
548+
render: function () {
549+
return <RouteHandler/>;
550+
}
551+
});
552+
538553
var Spoon = React.createClass({
539554
statics: {
540555
willTransitionTo: function (transition, params, query) {
@@ -543,12 +558,13 @@ describe('Router', function () {
543558
}
544559

545560
expect(query['filter']).toEqual('second');
546-
expect(fromCallbackExecuted).toBe(true);
561+
expect(fromKnifeCalled).toBe(true);
562+
expect(fromSpoonCalled).toBe(true);
547563
done();
548564
},
549565

550566
willTransitionFrom: function (transition, element) {
551-
fromCallbackExecuted = true;
567+
fromSpoonCalled = true;
552568
}
553569
},
554570

@@ -558,7 +574,7 @@ describe('Router', function () {
558574
});
559575

560576
var routes = (
561-
<Route handler={Nested} path='/'>
577+
<Route handler={Knife}>
562578
<Route name="spoon" handler={Spoon}/>
563579
</Route>
564580
);

modules/utils/createRouter.js

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -91,22 +91,32 @@ function createMatch(route, params) {
9191
return { routes: [ route ], params: params };
9292
}
9393

94-
function hasMatch(routes, route, prevParams, nextParams) {
94+
function hasProperties(object, properties) {
95+
for (var propertyName in properties)
96+
if (properties.hasOwnProperty(propertyName) && object[propertyName] !== properties[propertyName])
97+
return false;
98+
99+
return true;
100+
}
101+
102+
function hasMatch(routes, route, prevParams, nextParams, prevQuery, nextQuery) {
95103
return routes.some(function (r) {
96104
if (r !== route)
97105
return false;
98106

99107
var paramNames = route.paramNames;
100108
var paramName;
101109

110+
// Ensure that all params the route cares about did not change.
102111
for (var i = 0, len = paramNames.length; i < len; ++i) {
103112
paramName = paramNames[i];
104113

105114
if (nextParams[paramName] !== prevParams[paramName])
106115
return false;
107116
}
108117

109-
return true;
118+
// Ensure the query hasn't changed.
119+
return hasProperties(prevQuery, nextQuery) && hasProperties(nextQuery, prevQuery);
110120
});
111121
}
112122

@@ -335,6 +345,7 @@ function createRouter(options) {
335345

336346
var prevRoutes = state.routes || [];
337347
var prevParams = state.params || {};
348+
var prevQuery = state.query || {};
338349

339350
var nextRoutes = match.routes || [];
340351
var nextParams = match.params || {};
@@ -343,27 +354,17 @@ function createRouter(options) {
343354
var fromRoutes, toRoutes;
344355
if (prevRoutes.length) {
345356
fromRoutes = prevRoutes.filter(function (route) {
346-
return !hasMatch(nextRoutes, route, prevParams, nextParams);
357+
return !hasMatch(nextRoutes, route, prevParams, nextParams, prevQuery, nextQuery);
347358
});
348359

349360
toRoutes = nextRoutes.filter(function (route) {
350-
return !hasMatch(prevRoutes, route, prevParams, nextParams);
361+
return !hasMatch(prevRoutes, route, prevParams, nextParams, prevQuery, nextQuery);
351362
});
352363
} else {
353364
fromRoutes = [];
354365
toRoutes = nextRoutes;
355366
}
356367

357-
// If routes' hooks arrays are empty, then we transition to current route.
358-
// But path is somehow still get changed.
359-
// That could be only because of route query changes.
360-
// Need to push current route to routes' hooks arrays.
361-
if (!toRoutes.length && !fromRoutes.length) {
362-
var currentRoute = state.routes[state.routes.length-1];
363-
fromRoutes = [currentRoute];
364-
toRoutes = [currentRoute];
365-
}
366-
367368
var transition = new Transition(path, this.replaceWith.bind(this, path));
368369
pendingTransition = transition;
369370

0 commit comments

Comments
 (0)