fix: don't merge params on navigation

BREAKING CHANGE: Previous versions of React Navigation merged params on navigation which caused confusion. This commit changes params not to be merged.

The old behaviour can still be achieved by passing `merge: true` explicitly:

```js
CommonActions.navigate({
  name: 'bar',
  params: { fruit: 'orange' },
  merge: true,
})
```
`initialParams` specified for the screen are always merged.
This commit is contained in:
Satyajit Sahoo
2020-11-11 23:11:31 +01:00
parent ebab518352
commit 366d0181dc
6 changed files with 172 additions and 183 deletions

View File

@@ -353,7 +353,7 @@ export default function useNavigationBuilder<
if (
typeof route.params.state === 'object' &&
route.params.state != null &&
route.params.state !== previousParams?.state
route.params !== previousParams
) {
// If the route was updated with new state, we should reset to it
action = CommonActions.reset(route.params.state);
@@ -362,7 +362,6 @@ export default function useNavigationBuilder<
((route.params.initial === false && isFirstStateInitialization) ||
route.params !== previousParams)
) {
// FIXME: Since params are merged, `route.params.params` might contain params from an older route
// If the route was updated with new screen name and/or params, we should navigate there
action = CommonActions.navigate(route.params.screen, route.params.params);
}

View File

@@ -39,9 +39,9 @@ export function goBack(): Action {
}
export function navigate(
route:
| { key: string; params?: object }
| { name: string; key?: string; params?: object }
options:
| { key: string; params?: object; merge?: boolean }
| { name: string; key?: string; params?: object; merge?: boolean }
): Action;
// eslint-disable-next-line no-redeclare
export function navigate(name: string, params?: object): Action;

View File

@@ -401,7 +401,17 @@ export default function StackRouter(options: StackRouterOptions) {
let params;
if (action.payload.merge === false) {
if (action.payload.merge) {
params =
action.payload.params !== undefined ||
routeParamList[route.name] !== undefined
? {
...routeParamList[route.name],
...route.params,
...action.payload.params,
}
: route.params;
} else {
params =
routeParamList[route.name] !== undefined
? {
@@ -409,13 +419,6 @@ export default function StackRouter(options: StackRouterOptions) {
...action.payload.params,
}
: action.payload.params;
} else {
params = action.payload.params
? {
...route.params,
...action.payload.params,
}
: route.params;
}
return {

View File

@@ -294,40 +294,35 @@ export default function TabRouter({
return changeIndex(
{
...state,
routes:
action.payload.params !== undefined
? state.routes.map((route, i) => {
if (i !== index) {
return route;
}
routes: state.routes.map((route, i) => {
if (i !== index) {
return route;
}
let params;
let params;
if (
action.type === 'NAVIGATE' &&
action.payload.merge === false
) {
params =
routeParamList[route.name] !== undefined
? {
...routeParamList[route.name],
...action.payload.params,
}
: action.payload.params;
} else {
params = action.payload.params
? {
...route.params,
...action.payload.params,
}
: route.params;
}
if (action.type === 'NAVIGATE' && action.payload.merge) {
params =
action.payload.params !== undefined ||
routeParamList[route.name] !== undefined
? {
...routeParamList[route.name],
...route.params,
...action.payload.params,
}
: route.params;
} else {
params =
routeParamList[route.name] !== undefined
? {
...routeParamList[route.name],
...action.payload.params,
}
: action.payload.params;
}
return params !== route.params
? { ...route, params }
: route;
})
: state.routes,
return params !== route.params ? { ...route, params } : route;
}),
},
index,
backBehavior,

View File

@@ -1010,11 +1010,13 @@ it('changes index on focus change', () => {
expect(router.getStateForRouteFocus(state, 'qux-0')).toEqual(state);
});
it('merges params on navigate to an existing screen', () => {
it("doesn't merge params on navigate to an existing screen", () => {
const router = StackRouter({});
const options = {
routeNames: ['baz', 'bar', 'qux'],
routeParamList: {},
routeParamList: {
bar: { color: 'test' },
},
};
expect(
@@ -1042,7 +1044,7 @@ it('merges params on navigate to an existing screen', () => {
routeNames: ['baz', 'bar', 'qux'],
routes: [
{ key: 'baz', name: 'baz' },
{ key: 'bar', name: 'bar', params: { answer: 42 } },
{ key: 'bar', name: 'bar', params: { color: 'test' } },
],
});
@@ -1070,16 +1072,17 @@ it('merges params on navigate to an existing screen', () => {
routeNames: ['baz', 'bar', 'qux'],
routes: [
{ key: 'baz', name: 'baz' },
{ key: 'bar', name: 'bar', params: { answer: 42, fruit: 'orange' } },
{ key: 'bar', name: 'bar', params: { color: 'test', fruit: 'orange' } },
],
});
});
it("doesn't merge params on navigate to an existing screen if merge: false", () => {
it('merges params on navigate to an existing screen if merge: true', () => {
const router = StackRouter({});
const options = {
routeNames: ['baz', 'bar', 'qux'],
routeParamList: {
bar: { color: 'test' },
baz: { foo: 12 },
},
};
@@ -1098,13 +1101,11 @@ it("doesn't merge params on navigate to an existing screen if merge: false", ()
{ key: 'qux', name: 'qux' },
],
},
{
type: 'NAVIGATE',
payload: {
name: 'bar',
merge: false,
},
},
CommonActions.navigate({
name: 'bar',
merge: true,
}),
options
)
).toEqual({
@@ -1115,7 +1116,7 @@ it("doesn't merge params on navigate to an existing screen if merge: false", ()
routeNames: ['baz', 'bar', 'qux'],
routes: [
{ key: 'baz', name: 'baz' },
{ key: 'bar', name: 'bar' },
{ key: 'bar', name: 'bar', params: { color: 'test', answer: 42 } },
],
});
@@ -1132,14 +1133,11 @@ it("doesn't merge params on navigate to an existing screen if merge: false", ()
{ key: 'bar', name: 'bar', params: { answer: 42 } },
],
},
{
type: 'NAVIGATE',
payload: {
name: 'bar',
params: { fruit: 'orange' },
merge: false,
},
},
CommonActions.navigate({
name: 'bar',
params: { fruit: 'orange' },
merge: true,
}),
options
)
).toEqual({
@@ -1150,7 +1148,11 @@ it("doesn't merge params on navigate to an existing screen if merge: false", ()
routeNames: ['baz', 'bar', 'qux'],
routes: [
{ key: 'baz', name: 'baz' },
{ key: 'bar', name: 'bar', params: { fruit: 'orange' } },
{
key: 'bar',
name: 'bar',
params: { color: 'test', fruit: 'orange', answer: 42 },
},
],
});
@@ -1167,14 +1169,11 @@ it("doesn't merge params on navigate to an existing screen if merge: false", ()
{ key: 'bar', name: 'bar', params: { answer: 42 } },
],
},
{
type: 'NAVIGATE',
payload: {
name: 'baz',
params: { color: 'black' },
merge: false,
},
},
CommonActions.navigate({
name: 'baz',
params: { color: 'black' },
merge: true,
}),
options
)
).toEqual({
@@ -1183,6 +1182,12 @@ it("doesn't merge params on navigate to an existing screen if merge: false", ()
key: 'root',
index: 0,
routeNames: ['baz', 'bar', 'qux'],
routes: [{ key: 'baz', name: 'baz', params: { foo: 12, color: 'black' } }],
routes: [
{
key: 'baz',
name: 'baz',
params: { foo: 12, test: 99, color: 'black' },
},
],
});
});

View File

@@ -1108,11 +1108,13 @@ it('updates route key history on focus change', () => {
]);
});
it('merges params on navigate to an existing screen', () => {
it("doesn't merge params on navigate to an existing screen", () => {
const router = TabRouter({});
const options = {
routeNames: ['baz', 'bar', 'qux'],
routeParamList: {},
routeParamList: {
qux: { color: 'indigo' },
},
};
expect(
@@ -1141,93 +1143,7 @@ it('merges params on navigate to an existing screen', () => {
routeNames: ['baz', 'bar', 'qux'],
routes: [
{ key: 'baz', name: 'baz' },
{ key: 'bar', name: 'bar', params: { answer: 42 } },
{ key: 'qux', name: 'qux' },
],
history: [
{ type: 'route', key: 'baz' },
{ type: 'route', key: 'bar' },
],
});
expect(
router.getStateForAction(
{
stale: false,
type: 'tab',
key: 'root',
index: 1,
routeNames: ['baz', 'bar', 'qux'],
routes: [
{ key: 'baz', name: 'baz' },
{ key: 'bar', name: 'bar', params: { answer: 42 } },
{ key: 'qux', name: 'qux' },
],
history: [{ type: 'route', key: 'baz' }],
},
CommonActions.navigate('bar', { fruit: 'orange' }),
options
)
).toEqual({
stale: false,
type: 'tab',
key: 'root',
index: 1,
routeNames: ['baz', 'bar', 'qux'],
routes: [
{ key: 'baz', name: 'baz' },
{ key: 'bar', name: 'bar', params: { answer: 42, fruit: 'orange' } },
{ key: 'qux', name: 'qux' },
],
history: [
{ type: 'route', key: 'baz' },
{ type: 'route', key: 'bar' },
],
});
});
it("doesn't merge params on navigate to an existing screen if merge: false", () => {
const router = TabRouter({});
const options = {
routeNames: ['baz', 'bar', 'qux'],
routeParamList: {
qux: { color: 'indigo' },
},
};
expect(
router.getStateForAction(
{
stale: false,
type: 'tab',
key: 'root',
index: 1,
routeNames: ['baz', 'bar', 'qux'],
routes: [
{ key: 'baz', name: 'baz' },
{ key: 'bar', name: 'bar', params: { answer: 42 } },
{ key: 'qux', name: 'qux' },
],
history: [{ type: 'route', key: 'baz' }],
},
{
type: 'NAVIGATE',
payload: {
name: 'bar',
merge: false,
},
},
options
)
).toEqual({
stale: false,
type: 'tab',
key: 'root',
index: 1,
routeNames: ['baz', 'bar', 'qux'],
routes: [
{ key: 'baz', name: 'baz' },
{ key: 'bar', name: 'bar', params: { answer: 42 } },
{ key: 'bar', name: 'bar' },
{ key: 'qux', name: 'qux' },
],
history: [
@@ -1251,14 +1167,7 @@ it("doesn't merge params on navigate to an existing screen if merge: false", ()
],
history: [{ type: 'route', key: 'baz' }],
},
{
type: 'NAVIGATE',
payload: {
name: 'bar',
params: { fruit: 'orange' },
merge: false,
},
},
CommonActions.navigate('bar', { fruit: 'orange' }),
options
)
).toEqual({
@@ -1293,14 +1202,7 @@ it("doesn't merge params on navigate to an existing screen if merge: false", ()
],
history: [{ type: 'route', key: 'baz' }],
},
{
type: 'NAVIGATE',
payload: {
name: 'qux',
params: { test: 12 },
merge: false,
},
},
CommonActions.navigate('qux', { test: 12 }),
options
)
).toEqual({
@@ -1321,7 +1223,7 @@ it("doesn't merge params on navigate to an existing screen if merge: false", ()
});
});
it('merges params on jump to an existing screen', () => {
it('merges params on navigate to an existing screen if merge: true', () => {
const router = TabRouter({});
const options = {
routeNames: ['baz', 'bar', 'qux'],
@@ -1343,7 +1245,10 @@ it('merges params on jump to an existing screen', () => {
],
history: [{ type: 'route', key: 'baz' }],
},
TabActions.jumpTo('bar'),
CommonActions.navigate({
name: 'bar',
merge: true,
}),
options
)
).toEqual({
@@ -1378,7 +1283,11 @@ it('merges params on jump to an existing screen', () => {
],
history: [{ type: 'route', key: 'baz' }],
},
TabActions.jumpTo('bar', { fruit: 'orange' }),
CommonActions.navigate({
name: 'bar',
params: { fruit: 'orange' },
merge: true,
}),
options
)
).toEqual({
@@ -1398,3 +1307,81 @@ it('merges params on jump to an existing screen', () => {
],
});
});
it("doesn't merge params on jump to an existing screen", () => {
const router = TabRouter({});
const options = {
routeNames: ['baz', 'bar', 'qux'],
routeParamList: {},
};
expect(
router.getStateForAction(
{
stale: false,
type: 'tab',
key: 'root',
index: 1,
routeNames: ['baz', 'bar', 'qux'],
routes: [
{ key: 'baz', name: 'baz' },
{ key: 'bar', name: 'bar', params: { answer: 42 } },
{ key: 'qux', name: 'qux' },
],
history: [{ type: 'route', key: 'baz' }],
},
TabActions.jumpTo('bar'),
options
)
).toEqual({
stale: false,
type: 'tab',
key: 'root',
index: 1,
routeNames: ['baz', 'bar', 'qux'],
routes: [
{ key: 'baz', name: 'baz' },
{ key: 'bar', name: 'bar' },
{ key: 'qux', name: 'qux' },
],
history: [
{ type: 'route', key: 'baz' },
{ type: 'route', key: 'bar' },
],
});
expect(
router.getStateForAction(
{
stale: false,
type: 'tab',
key: 'root',
index: 1,
routeNames: ['baz', 'bar', 'qux'],
routes: [
{ key: 'baz', name: 'baz' },
{ key: 'bar', name: 'bar', params: { answer: 42 } },
{ key: 'qux', name: 'qux' },
],
history: [{ type: 'route', key: 'baz' }],
},
TabActions.jumpTo('bar', { fruit: 'orange' }),
options
)
).toEqual({
stale: false,
type: 'tab',
key: 'root',
index: 1,
routeNames: ['baz', 'bar', 'qux'],
routes: [
{ key: 'baz', name: 'baz' },
{ key: 'bar', name: 'bar', params: { fruit: 'orange' } },
{ key: 'qux', name: 'qux' },
],
history: [
{ type: 'route', key: 'baz' },
{ type: 'route', key: 'bar' },
],
});
});