Skip to content

Commit

Permalink
StackRouter to return null on idempotent navigation (#3793)
Browse files Browse the repository at this point in the history
This new behavior indicates that the action has been handled, but the state has not changed.
  • Loading branch information
ericvicenti authored and brentvatne committed Mar 19, 2018
1 parent aa362ea commit 577d99c
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 7 deletions.
2 changes: 1 addition & 1 deletion src/routers/StackRouter.js
Expand Up @@ -209,7 +209,7 @@ export default (routeConfigs, stackConfig = {}) => {
if (action.type !== NavigationActions.PUSH && lastRouteIndex !== -1) { if (action.type !== NavigationActions.PUSH && lastRouteIndex !== -1) {
// If index is unchanged and params are not being set, leave state identity intact // If index is unchanged and params are not being set, leave state identity intact
if (state.index === lastRouteIndex && !action.params) { if (state.index === lastRouteIndex && !action.params) {
return state; return null;
} }


// Remove the now unused routes at the tail of the routes array // Remove the now unused routes at the tail of the routes array
Expand Down
22 changes: 16 additions & 6 deletions src/routers/__tests__/StackRouter-test.js
Expand Up @@ -539,8 +539,7 @@ describe('StackRouter', () => {
NavigationActions.navigate({ routeName: 'bar' }), NavigationActions.navigate({ routeName: 'bar' }),
barState barState
); );
expect(navigateOnBarState.index).toEqual(1); expect(navigateOnBarState).toEqual(null);
expect(navigateOnBarState.routes[1].routeName).toEqual('bar');
}); });


test('Navigate focuses given routeName if already active in stack', () => { test('Navigate focuses given routeName if already active in stack', () => {
Expand Down Expand Up @@ -640,8 +639,7 @@ describe('StackRouter', () => {
NavigationActions.navigate({ routeName: 'foo', key: 'foo' }), NavigationActions.navigate({ routeName: 'foo', key: 'foo' }),
initState initState
); );
expect(pushedState.index).toEqual(0); expect(pushedState).toEqual(null);
expect(pushedState.routes[0].routeName).toEqual('foo');
}); });


test('Navigate with key is idempotent', () => { test('Navigate with key is idempotent', () => {
Expand All @@ -660,8 +658,20 @@ describe('StackRouter', () => {
NavigationActions.navigate({ routeName: 'bar', key: 'a' }), NavigationActions.navigate({ routeName: 'bar', key: 'a' }),
pushedState pushedState
); );
expect(pushedTwiceState.index).toEqual(1); expect(pushedTwiceState).toEqual(null);
expect(pushedTwiceState.routes[1].routeName).toEqual('bar'); });

test('Navigate to current routeName returns null to indicate handled action', () => {
const TestRouter = StackRouter({
foo: { screen: () => <div /> },
bar: { screen: () => <div /> },
});
const initState = TestRouter.getStateForAction(NavigationActions.init());
const navigatedState = TestRouter.getStateForAction(
NavigationActions.navigate({ routeName: 'foo' }),
initState
);
expect(navigatedState).toBe(null);
}); });


test('Push behaves like navigate, except for key', () => { test('Push behaves like navigate, except for key', () => {
Expand Down

0 comments on commit 577d99c

Please sign in to comment.