Skip to content

Commit 67002e8

Browse files
Hedger WangFacebook Github Bot 4
authored andcommitted
Clean up NavigationStateUtils
Summary: == API Breaking Change == - Add unit tests to ensure that NavigationStateUtils does the right thing. - Remove the logics that lets NavigationStateUtils accept empty value as input and return a new state. - Remove the method `NavigationStateUtils.getParent`, `NavigationStateUtils.set`. These methods are rarely used and they can be replaced by other methods. Reviewed By: ericvicenti Differential Revision: D3374934 fbshipit-source-id: 0fdf538d014d7c5b4aa1f15a0ee8db9dc91e33cd
1 parent 30e9c40 commit 67002e8

File tree

3 files changed

+266
-154
lines changed

3 files changed

+266
-154
lines changed

Examples/UIExplorer/UIExplorerNavigationReducer.js

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,13 +44,26 @@ export type UIExplorerNavigationState = {
4444

4545
const defaultGetReducerForState = (initialState) => (state) => state || initialState;
4646

47+
function getNavigationState(state: any): ?NavigationState {
48+
if (
49+
(state instanceof Object) &&
50+
(state.routes instanceof Array) &&
51+
(state.routes[0] !== undefined) &&
52+
(typeof state.index === 'number') &&
53+
(state.routes[state.index] !== undefined)
54+
) {
55+
return state;
56+
}
57+
return null;
58+
}
59+
4760
function StackReducer({initialState, getReducerForState, getPushedReducerForAction}: any): Function {
4861
const getReducerForStateWithDefault = getReducerForState || defaultGetReducerForState;
4962
return function (lastState: ?NavigationState, action: any): NavigationState {
5063
if (!lastState) {
5164
return initialState;
5265
}
53-
const lastParentState = NavigationStateUtils.getParent(lastState);
66+
const lastParentState = getNavigationState(lastState);
5467
if (!lastParentState) {
5568
return lastState;
5669
}

Libraries/NavigationExperimental/NavigationStateUtils.js

Lines changed: 125 additions & 112 deletions
Original file line numberDiff line numberDiff line change
@@ -18,166 +18,180 @@ import type {
1818
NavigationState,
1919
} from 'NavigationTypeDefinition';
2020

21-
function getParent(state: NavigationState): ?NavigationState {
22-
if (
23-
(state instanceof Object) &&
24-
(state.routes instanceof Array) &&
25-
(state.routes[0] !== undefined) &&
26-
(typeof state.index === 'number') &&
27-
(state.routes[state.index] !== undefined)
28-
) {
29-
return state;
30-
}
31-
return null;
32-
}
21+
/**
22+
* Utilities to perform atomic operation with navigate state and routes.
23+
*/
3324

25+
/**
26+
* Gets a route by key
27+
*/
3428
function get(state: NavigationState, key: string): ?NavigationRoute {
35-
const parentState = getParent(state);
36-
if (!parentState) {
37-
return null;
38-
}
39-
const childState = parentState.routes.find(child => child.key === key);
40-
return childState || null;
29+
return state.routes.find(route => route.key === key) || null;
4130
}
4231

43-
function indexOf(state: NavigationState, key: string): ?number {
44-
const parentState = getParent(state);
45-
if (!parentState) {
46-
return null;
47-
}
48-
const index = parentState.routes.map(child => child.key).indexOf(key);
49-
if (index === -1) {
50-
return null;
51-
}
52-
return index;
32+
/**
33+
* Returns the first index at which a given route's key can be found in the
34+
* routes of the navigation state, or -1 if it is not present.
35+
*/
36+
function indexOf(state: NavigationState, key: string): number {
37+
return state.routes.map(route => route.key).indexOf(key);
38+
}
39+
40+
/**
41+
* Returns `true` at which a given route's key can be found in the
42+
* routes of the navigation state.
43+
*/
44+
function has(state: NavigationState, key: string): boolean {
45+
return !!state.routes.some(route => route.key === key);
5346
}
5447

55-
function push(state: NavigationState, newChildState: NavigationRoute): NavigationState {
56-
var lastChildren: Array<NavigationRoute> = state.routes;
48+
/**
49+
* Pushes a new route into the navigation state.
50+
* Note that this moves the index to the positon to where the last route in the
51+
* stack is at.
52+
*/
53+
function push(state: NavigationState, route: NavigationRoute): NavigationState {
54+
invariant(
55+
indexOf(state, route.key) === -1,
56+
'should not push route with duplicated key %s',
57+
route.key,
58+
);
59+
60+
const routes = [
61+
...state.routes,
62+
route,
63+
];
64+
5765
return {
5866
...state,
59-
routes: [
60-
...lastChildren,
61-
newChildState,
62-
],
63-
index: lastChildren.length,
67+
index: routes.length - 1,
68+
routes,
6469
};
6570
}
6671

72+
/**
73+
* Pops out a route from the navigation state.
74+
* Note that this moves the index to the positon to where the last route in the
75+
* stack is at.
76+
*/
6777
function pop(state: NavigationState): NavigationState {
6878
if (state.index <= 0) {
79+
// [Note]: Over-popping does not throw error. Instead, it will be no-op.
6980
return state;
7081
}
71-
const lastChildren = state.routes;
82+
const routes = state.routes.slice(0, -1);
7283
return {
7384
...state,
74-
routes: lastChildren.slice(0, lastChildren.length - 1),
75-
index: lastChildren.length - 2,
85+
index: routes.length - 1,
86+
routes,
7687
};
7788
}
7889

79-
function reset(state: NavigationState, nextChildren: ?Array<NavigationRoute>, nextIndex: ?number): NavigationState {
80-
const parentState = getParent(state);
81-
if (!parentState) {
82-
return state;
83-
}
84-
const routes = nextChildren || parentState.routes;
85-
const index = nextIndex == null ? parentState.index : nextIndex;
86-
if (routes === parentState.routes && index === parentState.index) {
90+
/**
91+
* Sets the focused route of the navigation state by index.
92+
*/
93+
function jumpToIndex(state: NavigationState, index: number): NavigationState {
94+
if (index === state.index) {
8795
return state;
8896
}
97+
98+
invariant(!!state.routes[index], 'invalid index %s to jump to', index);
99+
89100
return {
90-
...parentState,
91-
routes,
101+
...state,
92102
index,
93103
};
94104
}
95105

96-
function set(state: ?NavigationState, key: string, nextChildren: Array<NavigationRoute>, nextIndex: number): NavigationState {
97-
if (!state) {
98-
return {
99-
routes: nextChildren,
100-
index: nextIndex,
101-
};
102-
}
103-
const parentState = getParent(state);
104-
if (!parentState) {
105-
return {
106-
routes: nextChildren,
107-
index: nextIndex,
108-
};
109-
}
110-
if (nextChildren === parentState.routes && nextIndex === parentState.index) {
111-
return parentState;
112-
}
113-
return {
114-
...parentState,
115-
routes: nextChildren,
116-
index: nextIndex,
117-
};
106+
/**
107+
* Sets the focused route of the navigation state by key.
108+
*/
109+
function jumpTo(state: NavigationState, key: string): NavigationState {
110+
const index = indexOf(state, key);
111+
return jumpToIndex(state, index);
118112
}
119113

120-
function jumpToIndex(state: NavigationState, index: number): NavigationState {
121-
const parentState = getParent(state);
122-
if (parentState && parentState.index === index) {
123-
return parentState;
124-
}
125-
return {
126-
...parentState,
127-
index,
128-
};
114+
/**
115+
* Replace a route by a key.
116+
* Note that this moves the index to the positon to where the new route in the
117+
* stack is at.
118+
*/
119+
function replaceAt(
120+
state: NavigationState,
121+
key: string,
122+
route: NavigationRoute,
123+
): NavigationState {
124+
const index = indexOf(state, key);
125+
return replaceAtIndex(state, index, route);
129126
}
130127

131-
function jumpTo(state: NavigationState, key: string): NavigationState {
132-
const parentState = getParent(state);
133-
if (!parentState) {
134-
return state;
135-
}
136-
const index = parentState.routes.indexOf(parentState.routes.find(child => child.key === key));
128+
/**
129+
* Replace a route by a index.
130+
* Note that this moves the index to the positon to where the new route in the
131+
* stack is at.
132+
*/
133+
function replaceAtIndex(
134+
state: NavigationState,
135+
index: number,
136+
route: NavigationRoute,
137+
): NavigationState {
137138
invariant(
138-
index !== -1,
139-
'Cannot find child with matching key in this NavigationRoute'
140-
);
141-
return {
142-
...parentState,
139+
!!state.routes[index],
140+
'invalid index %s for replacing route %s',
143141
index,
144-
};
145-
}
142+
route.key,
143+
);
146144

147-
function replaceAt(state: NavigationState, key: string, newState: NavigationRoute): NavigationState {
148-
const parentState = getParent(state);
149-
if (!parentState) {
145+
if (state.routes[index] === route) {
150146
return state;
151147
}
152-
const routes = [...parentState.routes];
153-
const index = parentState.routes.indexOf(parentState.routes.find(child => child.key === key));
154-
invariant(
155-
index !== -1,
156-
'Cannot find child with matching key in this NavigationRoute'
157-
);
158-
routes[index] = newState;
148+
149+
const routes = state.routes.slice();
150+
routes[index] = route;
151+
159152
return {
160-
...parentState,
153+
...state,
154+
index,
161155
routes,
162156
};
163157
}
164158

165-
function replaceAtIndex(state: NavigationState, index: number, newState: NavigationRoute): NavigationState {
166-
const parentState = getParent(state);
167-
if (!parentState) {
168-
return state;
159+
/**
160+
* Resets all routes.
161+
* Note that this moves the index to the positon to where the last route in the
162+
* stack is at if the param `index` isn't provided.
163+
*/
164+
function reset(
165+
state: NavigationState,
166+
routes: Array<NavigationRoute>,
167+
index?: number,
168+
): NavigationState {
169+
invariant(
170+
routes.length && Array.isArray(routes),
171+
'invalid routes to replace',
172+
);
173+
174+
const nextIndex: number = index === undefined ? routes.length - 1 : index;
175+
176+
if (state.routes.length === routes.length && state.index === nextIndex) {
177+
const compare = (route, ii) => routes[ii] === route;
178+
if (state.routes.every(compare)) {
179+
return state;
180+
}
169181
}
170-
const routes = [...parentState.routes];
171-
routes[index] = newState;
182+
183+
invariant(!!routes[nextIndex], 'invalid index %s to reset', nextIndex);
184+
172185
return {
173-
...parentState,
186+
...state,
187+
index: nextIndex,
174188
routes,
175189
};
176190
}
177191

178192
const NavigationStateUtils = {
179193
get: get,
180-
getParent,
194+
has,
181195
indexOf,
182196
jumpTo,
183197
jumpToIndex,
@@ -186,7 +200,6 @@ const NavigationStateUtils = {
186200
replaceAt,
187201
replaceAtIndex,
188202
reset,
189-
set: set,
190203
};
191204

192205
module.exports = NavigationStateUtils;

0 commit comments

Comments
 (0)