From e7014a62e3a7b7a4f447d23b0e188aa48eba8a7a Mon Sep 17 00:00:00 2001 From: "satyajit.happy" Date: Mon, 15 Jul 2019 04:35:21 +0200 Subject: [PATCH] refactor: remove code for cleaning up navigator It messes up with other state changes. We'll figure something out later --- example/index.tsx | 8 ++-- src/NavigationContainer.tsx | 13 ++++--- src/SceneView.tsx | 2 +- src/__tests__/index.test.tsx | 72 ++++-------------------------------- src/useNavigationBuilder.tsx | 6 --- 5 files changed, 20 insertions(+), 81 deletions(-) diff --git a/example/index.tsx b/example/index.tsx index 58a89f26..3e5c0669 100644 --- a/example/index.tsx +++ b/example/index.tsx @@ -58,7 +58,7 @@ const First = ({ Navigate with params ); @@ -80,7 +80,7 @@ const Second = ({ Push first ); @@ -104,7 +104,7 @@ const Fourth = ({ > Push first - @@ -127,7 +127,7 @@ const Fifth = ({ Push second ); diff --git a/src/NavigationContainer.tsx b/src/NavigationContainer.tsx index d89c3814..23301baa 100644 --- a/src/NavigationContainer.tsx +++ b/src/NavigationContainer.tsx @@ -33,16 +33,19 @@ export default function NavigationContainer({ NavigationState | PartialState | undefined >(initialState); - const initialMountRef = React.useRef(true); + const firstRenderRef = React.useRef(true); + const initialStateRef = React.useRef(initialState); const stateRef = React.useRef(state); - stateRef.current = state; + React.useLayoutEffect(() => { + stateRef.current = state; + }); React.useEffect(() => { - if (initialMountRef.current) { - initialMountRef.current = false; + if (firstRenderRef.current) { + firstRenderRef.current = false; - if (state === undefined) { + if (state === initialStateRef.current) { // Don't call the listener if we haven't initialized any state return; } diff --git a/src/SceneView.tsx b/src/SceneView.tsx index 4627c2ce..d8c91ca8 100644 --- a/src/SceneView.tsx +++ b/src/SceneView.tsx @@ -57,7 +57,7 @@ export default function SceneView(props: Props) { ), }); }, - [getState, route.key, setState] + [getState, route, setState] ); const context = React.useMemo( diff --git a/src/__tests__/index.test.tsx b/src/__tests__/index.test.tsx index 0e17664b..1fea97ea 100644 --- a/src/__tests__/index.test.tsx +++ b/src/__tests__/index.test.tsx @@ -237,7 +237,7 @@ it("doesn't update state if nothing changed", () => { const onStateChange = jest.fn(); - const element = ( + render( @@ -246,8 +246,6 @@ it("doesn't update state if nothing changed", () => { ); - render(element).update(element); - expect(onStateChange).toBeCalledTimes(0); }); @@ -271,7 +269,7 @@ it("doesn't update state if action wasn't handled", () => { const onStateChange = jest.fn(); - const element = ( + render( @@ -280,60 +278,9 @@ it("doesn't update state if action wasn't handled", () => { ); - render(element).update(element); - expect(onStateChange).toBeCalledTimes(0); }); -it('cleans up state when the navigator unmounts', () => { - const TestNavigator = (props: any) => { - const { navigation, descriptors } = useNavigationBuilder(MockRouter, props); - - return descriptors[ - navigation.state.routes[navigation.state.index].key - ].render(); - }; - - const FooScreen = (props: any) => { - React.useEffect(() => { - props.navigation.dispatch({ type: 'UPDATE' }); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, []); - - return null; - }; - - const onStateChange = jest.fn(); - - const element = ( - - - - - - - ); - - const root = render(element); - - root.update(element); - - expect(onStateChange).toBeCalledTimes(1); - expect(onStateChange).lastCalledWith({ - index: 0, - key: 'root', - routeNames: ['foo', 'bar'], - routes: [{ key: 'foo', name: 'foo' }, { key: 'bar', name: 'bar' }], - }); - - root.update( - - ); - - expect(onStateChange).toBeCalledTimes(2); - expect(onStateChange).lastCalledWith(undefined); -}); - it("lets parent handle the action if child didn't", () => { const ParentRouter: Router<{ type: string }> = { ...MockRouter, @@ -372,6 +319,7 @@ it("lets parent handle the action if child didn't", () => { const TestScreen = (props: any) => { React.useEffect(() => { props.navigation.dispatch({ type: 'REVERSE' }); + // eslint-disable-next-line react-hooks/exhaustive-deps }, []); @@ -380,7 +328,7 @@ it("lets parent handle the action if child didn't", () => { const onStateChange = jest.fn(); - const element = ( + render( {() => null} @@ -396,9 +344,7 @@ it("lets parent handle the action if child didn't", () => { ); - render(element).update(element); - - expect(onStateChange).toBeCalledTimes(2); + expect(onStateChange).toBeCalledTimes(1); expect(onStateChange).lastCalledWith({ index: 2, key: 'root', @@ -476,7 +422,7 @@ it('updates route params with setParams', () => { const onStateChange = jest.fn(); - const element = ( + render( @@ -485,8 +431,6 @@ it('updates route params with setParams', () => { ); - render(element); - act(() => setParams({ username: 'alice' })); expect(onStateChange).toBeCalledTimes(1); @@ -601,7 +545,7 @@ it("doesn't throw when direct children is Screen or empty element", () => { return null; }; - const element = ( + render( @@ -612,6 +556,4 @@ it("doesn't throw when direct children is Screen or empty element", () => { ); - - render(element).update(element); }); diff --git a/src/useNavigationBuilder.tsx b/src/useNavigationBuilder.tsx index 572d7916..1fe94e82 100644 --- a/src/useNavigationBuilder.tsx +++ b/src/useNavigationBuilder.tsx @@ -68,12 +68,6 @@ export default function useNavigationBuilder( setState, } = React.useContext(NavigationStateContext); - React.useEffect(() => { - // We need to clean up the state object when the navigator unmounts - return () => setState(undefined); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, []); - const getState = React.useCallback( (): NavigationState => router.getRehydratedState({