fix: remove the state property from route prop

BREAKING CHANGE: any code which relies on `route.state` will break.

Previous versions printed a warning on accessing `route.state`. This commit removes the property entirely. Accessing this property isn't safe since child navigator state isn't gurranteed to be in sync with parent navigator state and cause subtle bugs in apps.
This commit is contained in:
Satyajit Sahoo
2020-11-11 22:14:27 +01:00
parent b70ba66f24
commit ebab518352
3 changed files with 59 additions and 25 deletions

View File

@@ -1,4 +1,5 @@
import getFocusedRouteNameFromRoute from '../getFocusedRouteNameFromRoute';
import { CHILD_STATE } from '../useRouteCache';
it('gets undefined if there is no nested state', () => {
expect(getFocusedRouteNameFromRoute({ name: 'Home' })).toBe(undefined);
@@ -8,6 +9,7 @@ it('gets focused route name from nested state', () => {
expect(
getFocusedRouteNameFromRoute({
name: 'Home',
// @ts-expect-error: this isn't in the type defs
state: {
routes: [{ name: 'Article' }],
},
@@ -17,6 +19,7 @@ it('gets focused route name from nested state', () => {
expect(
getFocusedRouteNameFromRoute({
name: 'Home',
// @ts-expect-error: this isn't in the type defs
state: {
index: 1,
routes: [{ name: 'Article' }, { name: 'Chat' }, { name: 'Album' }],
@@ -27,6 +30,7 @@ it('gets focused route name from nested state', () => {
expect(
getFocusedRouteNameFromRoute({
name: 'Home',
// @ts-expect-error: this isn't in the type defs
state: {
routes: [{ name: 'Article' }, { name: 'Chat' }],
},
@@ -36,6 +40,7 @@ it('gets focused route name from nested state', () => {
expect(
getFocusedRouteNameFromRoute({
name: 'Home',
// @ts-expect-error: this isn't in the type defs
state: {
type: 'tab',
routes: [{ name: 'Article' }, { name: 'Chat' }],
@@ -44,6 +49,50 @@ it('gets focused route name from nested state', () => {
).toBe('Article');
});
it('gets focused route name from nested state with symbol', () => {
expect(
getFocusedRouteNameFromRoute({
name: 'Home',
// @ts-expect-error: this isn't in the type defs
[CHILD_STATE]: {
routes: [{ name: 'Article' }],
},
})
).toBe('Article');
expect(
getFocusedRouteNameFromRoute({
name: 'Home',
// @ts-expect-error: this isn't in the type defs
[CHILD_STATE]: {
index: 1,
routes: [{ name: 'Article' }, { name: 'Chat' }, { name: 'Album' }],
},
})
).toBe('Chat');
expect(
getFocusedRouteNameFromRoute({
name: 'Home',
// @ts-expect-error: this isn't in the type defs
[CHILD_STATE]: {
routes: [{ name: 'Article' }, { name: 'Chat' }],
},
})
).toBe('Chat');
expect(
getFocusedRouteNameFromRoute({
name: 'Home',
// @ts-expect-error: this isn't in the type defs
[CHILD_STATE]: {
type: 'tab',
routes: [{ name: 'Article' }, { name: 'Chat' }],
},
})
).toBe('Article');
});
it('gets nested screen in params if present', () => {
expect(
getFocusedRouteNameFromRoute({

View File

@@ -1,19 +1,11 @@
import type {
Route,
PartialState,
NavigationState,
} from '@react-navigation/routers';
import { SUPPRESS_STATE_ACCESS_WARNING } from './useRouteCache';
import type { Route } from '@react-navigation/routers';
import { CHILD_STATE } from './useRouteCache';
export default function getFocusedRouteNameFromRoute(
route: Partial<Route<string>> & { state?: PartialState<NavigationState> }
route: Partial<Route<string>>
): string | undefined {
SUPPRESS_STATE_ACCESS_WARNING.value = true;
const state = route.state;
SUPPRESS_STATE_ACCESS_WARNING.value = false;
// @ts-expect-error: this isn't in type definitions coz we want this private
const state = route[CHILD_STATE] ?? route.state;
const params = route.params as { screen?: unknown } | undefined;
const routeName = state

View File

@@ -13,7 +13,7 @@ type RouteCache = Map<Route<string>, RouteProp<ParamListBase, string>>;
* So we need a way to suppress the warning for those use cases.
* This is fine since they are internal utilities and this is not public API.
*/
export const SUPPRESS_STATE_ACCESS_WARNING = { value: false };
export const CHILD_STATE = Symbol('CHILD_STATE');
/**
* Hook to cache route props for each screen in the navigator.
@@ -37,18 +37,11 @@ export default function useRouteCache<State extends NavigationState>(
// If a cached route object already exists, reuse it
acc.set(route, previous);
} else {
const proxy = { ...route };
const { state, ...proxy } = route;
Object.defineProperty(proxy, 'state', {
get() {
if (!SUPPRESS_STATE_ACCESS_WARNING.value) {
console.warn(
"Accessing the 'state' property of the 'route' object is not supported. If you want to get the focused route name, use the 'getFocusedRouteNameFromRoute' helper instead: https://reactnavigation.org/docs/screen-options-resolution/#setting-parent-screen-options-based-on-child-navigators-state"
);
}
return route.state;
},
Object.defineProperty(proxy, CHILD_STATE, {
enumerable: false,
value: state,
});
acc.set(route, proxy);