Skip to content

Commit

Permalink
Consistent treatment of route keys (react-navigation#3474)
Browse files Browse the repository at this point in the history 
This problem was found and fixed by @matthargett and @jayphelps in react-navigation#3397. I’m just rebasing and cleaning a few things up
  • Loading branch information
@ericvicenti @sourcecode911
ericvicenti authored and sourcecode911 committed Mar 9, 2020
1 parent 60efd6a commit 0b13b50
Show file tree
Hide file tree
Showing 4 changed files with 168 additions and 44 deletions.
4 changes: 2 additions & 2 deletions src/navigators/__tests__/__snapshots__/StackNavigator-test.js.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ exports[`StackNavigator applies correct values when headerRight is present 1`] =
"key": "StackRouterRoot",
"routes": Array [
Object {
"key": "Init-id-0-1",
"key": "id-0-1",
"routeName": "Home",
},
],
Expand Down Expand Up @@ -347,7 +347,7 @@ exports[`StackNavigator renders successfully 1`] = `
"key": "StackRouterRoot",
"routes": Array [
Object {
"key": "Init-id-0-0",
"key": "id-0-0",
"routeName": "Home",
},
],
Expand Down
59 changes: 32 additions & 27 deletions src/routers/StackRouter.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,23 +101,32 @@ export default (routeConfigs, stackConfig = {}) => {
// Set up the initial state if needed
if (!state) {
let route = {};
if (
behavesLikePushAction(action) &&
childRouters[action.routeName] !== undefined
) {
const childRouter = childRouters[action.routeName];
// This is a push-like action, and childRouter will be a router or null if we are responsible for this routeName
if (behavesLikePushAction(action) && childRouter !== undefined) {
let childState = {};
// The router is null for normal leaf routes
if (childRouter !== null) {
const childAction =
action.action ||
NavigationActions.init({ params: action.params });
childState = childRouter.getStateForAction(childAction);
}
return {
key: 'StackRouterRoot',
isTransitioning: false,
index: 0,
routes: [
{
routeName: action.routeName,
params: action.params,
key: `Init-${generateKey()}`,
...childState,
key: action.key || generateKey(),
routeName: action.routeName,
},
],
};
}

if (initialChildRouter) {
route = initialChildRouter.getStateForAction(
NavigationActions.navigate({
Expand All @@ -135,11 +144,10 @@ export default (routeConfigs, stackConfig = {}) => {
};
route = {
...route,
routeName: initialRouteName,
key: `Init-${generateKey()}`,
...(params ? { params } : {}),
routeName: initialRouteName,
key: action.key || generateKey(),
};
// eslint-disable-next-line no-param-reassign
state = {
key: 'StackRouterRoot',
isTransitioning: false,
Expand Down Expand Up @@ -206,8 +214,8 @@ export default (routeConfigs, stackConfig = {}) => {
params: action.params,
// merge the child state in this order to allow params override
...childState,
key: action.newKey || generateKey(),
routeName: action.routeName,
key: action.newKey || generateKey(),
};
return { ...state, routes };
}
Expand Down Expand Up @@ -259,22 +267,22 @@ export default (routeConfigs, stackConfig = {}) => {
};
}
}
const key = action.key || generateKey();

if (childRouter) {
const childAction =
action.action || NavigationActions.init({ params: action.params });
route = {
params: action.params,
// merge the child state in this order to allow params override
...childRouter.getStateForAction(childAction),
key,
routeName: action.routeName,
key: action.key || generateKey(),
};
} else {
route = {
params: action.params,
key,
routeName: action.routeName,
key: action.key || generateKey(),
};
}
return {
Expand Down Expand Up @@ -326,11 +334,12 @@ export default (routeConfigs, stackConfig = {}) => {
routeToPush = navigatedChildRoute;
}
if (routeToPush) {
return StateUtils.push(state, {
const route = {
...routeToPush,
key: generateKey(),
routeName: childRouterName,
});
key: action.key || generateKey(),
};
return StateUtils.push(state, route);
}
}
}
Expand Down Expand Up @@ -369,20 +378,16 @@ export default (routeConfigs, stackConfig = {}) => {
...state,
routes: resetAction.actions.map(childAction => {
const router = childRouters[childAction.routeName];
let childState = {};
if (router) {
return {
...childAction,
...router.getStateForAction(childAction),
routeName: childAction.routeName,
key: generateKey(),
};
childState = router.getStateForAction(childAction);
}
const route = {
...childAction,
key: generateKey(),
return {
params: childAction.params,
...childState,
routeName: childAction.routeName,
key: childAction.key || generateKey(),
};
delete route.type;
return route;
}),
index: action.index,
};
Expand Down
13 changes: 6 additions & 7 deletions src/routers/__tests__/Routers-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import TabRouter from '../TabRouter';

import NavigationActions from '../../NavigationActions';
import addNavigationHelpers from '../../addNavigationHelpers';

import { _TESTING_ONLY_normalize_keys } from '../KeyGenerator';

beforeEach(() => {
Expand All @@ -19,7 +18,7 @@ const ROUTERS = {
StackRouter,
};

const dummyEventSubscriber = (name: string, handler: (*) => void) => ({
const dummyEventSubscriber = (name, handler) => ({
remove: () => {},
});

Expand Down Expand Up @@ -111,8 +110,8 @@ test('Handles no-op actions with tabs within stack router', () => {
type: NavigationActions.NAVIGATE,
routeName: 'Qux',
});
expect(state1.routes[0].key).toEqual('Init-id-0');
expect(state2.routes[0].key).toEqual('Init-id-1');
expect(state1.routes[0].key).toEqual('id-0');
expect(state2.routes[0].key).toEqual('id-1');
state1.routes[0].key = state2.routes[0].key;
expect(state1).toEqual(state2);
const state3 = TestRouter.getStateForAction(
Expand Down Expand Up @@ -140,7 +139,7 @@ test('Handles deep action', () => {
key: 'StackRouterRoot',
routes: [
{
key: 'Init-id-0',
key: 'id-0',
routeName: 'Bar',
},
],
Expand Down Expand Up @@ -180,8 +179,8 @@ test('Supports lazily-evaluated getScreen', () => {
immediate: true,
routeName: 'Qux',
});
expect(state1.routes[0].key).toEqual('Init-id-0');
expect(state2.routes[0].key).toEqual('Init-id-1');
expect(state1.routes[0].key).toEqual('id-0');
expect(state2.routes[0].key).toEqual('id-1');
state1.routes[0].key = state2.routes[0].key;
expect(state1).toEqual(state2);
const state3 = TestRouter.getStateForAction(
Expand Down
136 changes: 128 additions & 8 deletions src/routers/__tests__/StackRouter-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,7 @@ describe('StackRouter', () => {
index: 0,
isTransitioning: false,
key: 'StackRouterRoot',
routes: [{ key: 'Init-id-0', routeName: 'foo' }],
routes: [{ key: 'id-0', routeName: 'foo' }],
});
const pushedState = TestRouter.getStateForAction(
NavigationActions.navigate({ routeName: 'qux' }),
Expand Down Expand Up @@ -571,7 +571,7 @@ describe('StackRouter', () => {
key: 'StackRouterRoot',
routes: [
{
key: 'Init-id-0',
key: 'id-0',
routeName: 'Foo',
},
],
Expand Down Expand Up @@ -599,7 +599,7 @@ describe('StackRouter', () => {
key: 'StackRouterRoot',
routes: [
{
key: 'Init-id-0',
key: 'id-0',
routeName: 'Foo',
},
],
Expand Down Expand Up @@ -696,7 +696,7 @@ describe('StackRouter', () => {
key: 'StackRouterRoot',
routes: [
{
key: 'Init-id-0',
key: 'id-0',
routeName: 'Foo',
},
],
Expand Down Expand Up @@ -724,7 +724,7 @@ describe('StackRouter', () => {
key: 'StackRouterRoot',
routes: [
{
key: 'Init-id-0',
key: 'id-0',
routeName: 'Foo',
},
],
Expand Down Expand Up @@ -798,7 +798,7 @@ describe('StackRouter', () => {
key: 'StackRouterRoot',
routes: [
{
key: 'Init-id-0',
key: 'id-0',
routeName: 'Bar',
},
],
Expand Down Expand Up @@ -907,14 +907,14 @@ describe('StackRouter', () => {
{
type: NavigationActions.SET_PARAMS,
params: { name: 'foobar' },
key: 'Init-id-0',
key: 'id-0',
},
state
);
expect(state2 && state2.index).toEqual(0);
expect(state2 && state2.routes[0].routes[0].routes).toEqual([
{
key: 'Init-id-0',
key: 'id-0',
routeName: 'Quux',
params: { name: 'foobar' },
},
Expand Down Expand Up @@ -1133,6 +1133,126 @@ describe('StackRouter', () => {
]);
});

test('Handles the navigate action with params and nested StackRouter as a first action', () => {
const state = TestStackRouter.getStateForAction({
type: NavigationActions.NAVIGATE,
routeName: 'main',
params: {
code: 'test',
foo: 'bar',
},
action: {
type: NavigationActions.NAVIGATE,
routeName: 'profile',
params: {
id: '4',
code: 'test',
foo: 'bar',
},
action: {
type: NavigationActions.NAVIGATE,
routeName: 'list',
params: {
id: '10259959195',
code: 'test',
foo: 'bar',
},
},
},
});

expect(state).toEqual({
index: 0,
isTransitioning: false,
key: 'StackRouterRoot',
routes: [
{
index: 0,
isTransitioning: false,
key: 'id-2',
params: { code: 'test', foo: 'bar' },
routeName: 'main',
routes: [
{
index: 0,
isTransitioning: false,
key: 'id-1',
params: { code: 'test', foo: 'bar', id: '4' },
routeName: 'profile',
routes: [
{
key: 'id-0',
params: { code: 'test', foo: 'bar', id: '10259959195' },
routeName: 'list',
type: undefined,
},
],
},
],
},
],
});

const state2 = TestStackRouter.getStateForAction({
type: NavigationActions.NAVIGATE,
routeName: 'main',
params: {
code: '',
foo: 'bar',
},
action: {
type: NavigationActions.NAVIGATE,
routeName: 'profile',
params: {
id: '4',
code: '',
foo: 'bar',
},
action: {
type: NavigationActions.NAVIGATE,
routeName: 'list',
params: {
id: '10259959195',
code: '',
foo: 'bar',
},
},
},
});

expect(state2).toEqual({
index: 0,
isTransitioning: false,
key: 'StackRouterRoot',
routes: [
{
index: 0,
isTransitioning: false,
key: 'id-5',
params: { code: '', foo: 'bar' },
routeName: 'main',
routes: [
{
index: 0,
isTransitioning: false,
key: 'id-4',
params: { code: '', foo: 'bar', id: '4' },
routeName: 'profile',
routes: [
{
key: 'id-3',
params: { code: '', foo: 'bar', id: '10259959195' },
routeName: 'list',
type: undefined,
},
],
},
],
},
],
});
});

test('Handles the navigate action with params and nested TabRouter', () => {
const ChildNavigator = () => <div />;
ChildNavigator.router = TabRouter({
Expand Down

0 comments on commit 0b13b50

Please sign in to comment.