diff --git a/packages/core/src/BaseNavigationContainer.tsx b/packages/core/src/BaseNavigationContainer.tsx index 5cc696dbdf..50a9becb75 100644 --- a/packages/core/src/BaseNavigationContainer.tsx +++ b/packages/core/src/BaseNavigationContainer.tsx @@ -33,7 +33,6 @@ import { useEventEmitter } from './useEventEmitter'; import { useKeyedChildListeners } from './useKeyedChildListeners'; import { useNavigationIndependentTree } from './useNavigationIndependentTree'; import { useOptionsGetters } from './useOptionsGetters'; -import { ScheduleUpdateContext } from './useScheduleUpdate'; import { useSyncState } from './useSyncState'; type State = NavigationState | PartialState | undefined; @@ -103,10 +102,9 @@ export const BaseNavigationContainer = React.forwardRef( ); } - const [state, getState, setState, scheduleUpdate, flushUpdates] = - useSyncState(() => - getPartialState(initialState == null ? undefined : initialState) - ); + const [state, getState, setState] = useSyncState(() => + getPartialState(initialState == null ? undefined : initialState) + ); const isFirstMountRef = React.useRef(true); @@ -205,7 +203,7 @@ export const BaseNavigationContainer = React.forwardRef( isFocused: () => true, canGoBack, getParent: () => undefined, - getState: () => stateRef.current, + getState, getRootState, getCurrentRoute, getCurrentOptions, @@ -218,6 +216,7 @@ export const BaseNavigationContainer = React.forwardRef( getCurrentOptions, getCurrentRoute, getRootState, + getState, isReady, resetRoot, ] @@ -262,11 +261,6 @@ export const BaseNavigationContainer = React.forwardRef( [addListener, addKeyedListener, onDispatchAction, onOptionsChange] ); - const scheduleContext = React.useMemo( - () => ({ scheduleUpdate, flushUpdates }), - [scheduleUpdate, flushUpdates] - ); - const isInitialRef = React.useRef(true); const getIsInitial = React.useCallback(() => isInitialRef.current, []); @@ -294,13 +288,11 @@ export const BaseNavigationContainer = React.forwardRef( const onReadyRef = React.useRef(onReady); const onStateChangeRef = React.useRef(onStateChange); - const stateRef = React.useRef(state); React.useEffect(() => { isInitialRef.current = false; onStateChangeRef.current = onStateChange; onReadyRef.current = onReady; - stateRef.current = state; }); const onReadyCalledRef = React.useRef(false); @@ -448,26 +440,25 @@ export const BaseNavigationContainer = React.forwardRef( if (state !== previousState && stateForNextRouteNamesChange !== null) { setStateForNextRouteNamesChange(null); } + return ( - - - - - + + + + - - {children} - - - - - - + {children} + + + + + ); diff --git a/packages/core/src/__tests__/index.test.tsx b/packages/core/src/__tests__/index.test.tsx index a0fe7d1587..e247bcf058 100644 --- a/packages/core/src/__tests__/index.test.tsx +++ b/packages/core/src/__tests__/index.test.tsx @@ -430,8 +430,6 @@ it("doesn't update state if action wasn't handled", () => { }); it('cleans up state when the navigator unmounts', () => { - jest.useFakeTimers(); - const TestNavigator = (props: any) => { const { state, descriptors } = useNavigationBuilder(MockRouter, props); @@ -481,8 +479,6 @@ it('cleans up state when the navigator unmounts', () => { ); - act(() => jest.runAllTimers()); - expect(onStateChange).toHaveBeenCalledTimes(2); expect(onStateChange).toHaveBeenLastCalledWith(undefined); }); @@ -536,6 +532,72 @@ it('allows state updates by dispatching a function returning an action', () => { }); }); +it('re-initializes state once for conditional rendering', () => { + const TestNavigatorA = (props: any) => { + const { state, descriptors } = useNavigationBuilder(MockRouter, props); + + return descriptors[state.routes[state.index].key].render(); + }; + + const TestNavigatorB = (props: any) => { + const { state, descriptors } = useNavigationBuilder(MockRouter, props); + + return descriptors[state.routes[state.index].key].render(); + }; + + const onStateChange = jest.fn(); + + const navigation = createNavigationContainerRef(); + + const Test = ({ condition }: { condition: boolean }) => { + return ( + + {condition ? ( + + {() => null} + {() => null} + + ) : ( + + {() => null} + {() => null} + + )} + + ); + }; + + const root = render(); + + expect(onStateChange).toHaveBeenCalledTimes(0); + expect(navigation.getRootState()).toEqual({ + stale: false, + type: 'test', + index: 0, + key: '0', + routeNames: ['foo', 'bar'], + routes: [ + { key: 'foo', name: 'foo' }, + { key: 'bar', name: 'bar' }, + ], + }); + + root.update(); + + expect(onStateChange).toHaveBeenCalledTimes(1); + expect(onStateChange).toHaveBeenCalledWith({ + stale: false, + type: 'test', + index: 0, + key: '1', + routeNames: ['bar', 'baz'], + routes: [ + { key: 'bar', name: 'bar' }, + { key: 'baz', name: 'baz' }, + ], + }); +}); + it('updates route params with setParams', () => { const TestNavigator = (props: any) => { const { state, descriptors } = useNavigationBuilder(MockRouter, props); diff --git a/packages/core/src/useIsomorphicLayoutEffect.native.tsx b/packages/core/src/useIsomorphicLayoutEffect.native.tsx new file mode 100644 index 0000000000..b93b801c65 --- /dev/null +++ b/packages/core/src/useIsomorphicLayoutEffect.native.tsx @@ -0,0 +1,3 @@ +import { useLayoutEffect } from 'react'; + +export const useIsomorphicLayoutEffect = useLayoutEffect; diff --git a/packages/core/src/useIsomorphicLayoutEffect.tsx b/packages/core/src/useIsomorphicLayoutEffect.tsx new file mode 100644 index 0000000000..9dd3cd360c --- /dev/null +++ b/packages/core/src/useIsomorphicLayoutEffect.tsx @@ -0,0 +1,7 @@ +import { useEffect, useLayoutEffect } from 'react'; + +/** + * Use `useEffect` during SSR and `useLayoutEffect` in the browser to avoid warnings. + */ +export const useIsomorphicLayoutEffect = + typeof document !== 'undefined' ? useLayoutEffect : useEffect; diff --git a/packages/core/src/useNavigationBuilder.tsx b/packages/core/src/useNavigationBuilder.tsx index cf0c672994..627f382144 100644 --- a/packages/core/src/useNavigationBuilder.tsx +++ b/packages/core/src/useNavigationBuilder.tsx @@ -38,13 +38,13 @@ import { type ScreenConfigWithParent, useDescriptors } from './useDescriptors'; import { useEventEmitter } from './useEventEmitter'; import { useFocusedListenersChildrenAdapter } from './useFocusedListenersChildrenAdapter'; import { useFocusEvents } from './useFocusEvents'; +import { useIsomorphicLayoutEffect } from './useIsomorphicLayoutEffect'; import { useKeyedChildListeners } from './useKeyedChildListeners'; import { useNavigationHelpers } from './useNavigationHelpers'; import { useOnAction } from './useOnAction'; import { useOnGetState } from './useOnGetState'; import { useOnRouteFocus } from './useOnRouteFocus'; import { useRegisterNavigator } from './useRegisterNavigator'; -import { useScheduleUpdate } from './useScheduleUpdate'; // This is to make TypeScript compiler happy PrivateValueStore; @@ -339,12 +339,7 @@ export function useNavigationBuilder< const stateCleanedUp = React.useRef(false); - const cleanUpState = React.useCallback(() => { - setCurrentState(undefined); - stateCleanedUp.current = true; - }, [setCurrentState]); - - const setState = React.useCallback( + const setState = useLatestCallback( (state: NavigationState | PartialState | undefined) => { if (stateCleanedUp.current) { // State might have been already cleaned up due to unmount @@ -352,9 +347,9 @@ export function useNavigationBuilder< // This would lead to old data preservation on main navigator unmount return; } + setCurrentState(state); - }, - [setCurrentState] + } ); const [initializedState, isFirstStateInitialization] = React.useMemo(() => { @@ -547,7 +542,7 @@ export function useNavigationBuilder< const shouldUpdate = state !== nextState; - useScheduleUpdate(() => { + useIsomorphicLayoutEffect(() => { if (shouldUpdate) { // If the state needs to be updated, we'll schedule an update setState(nextState); @@ -571,31 +566,21 @@ export function useNavigationBuilder< return () => { // We need to clean up state for this navigator on unmount - // We do it in a timeout because we need to detect if another navigator mounted in the meantime - // For example, if another navigator has started rendering, we should skip cleanup - // Otherwise, our cleanup step will cleanup state for the other navigator and re-initialize it - setTimeout(() => { - if (getCurrentState() !== undefined && getKey() === navigatorKey) { - cleanUpState(); - } - }, 0); + if (getCurrentState() !== undefined && getKey() === navigatorKey) { + setCurrentState(undefined); + stateCleanedUp.current = true; + } }; // eslint-disable-next-line react-hooks/exhaustive-deps }, []); - // We initialize this ref here to avoid a new getState getting initialized - // whenever initializedState changes. We want getState to have access to the - // latest initializedState, but don't need it to change when that happens - const initializedStateRef = React.useRef(); - initializedStateRef.current = initializedState; - - const getState = React.useCallback((): State => { - const currentState = getCurrentState(); + const getState = useLatestCallback((): State => { + const currentState = shouldUpdate ? nextState : getCurrentState(); - return isStateInitialized(currentState) - ? (currentState as State) - : (initializedStateRef.current as State); - }, [getCurrentState, isStateInitialized]); + return ( + isStateInitialized(currentState) ? currentState : initializedState + ) as State; + }); const emitter = useEventEmitter>((e) => { const routeNames = []; diff --git a/packages/core/src/useScheduleUpdate.tsx b/packages/core/src/useScheduleUpdate.tsx deleted file mode 100644 index a7f1334f5d..0000000000 --- a/packages/core/src/useScheduleUpdate.tsx +++ /dev/null @@ -1,32 +0,0 @@ -import * as React from 'react'; - -const MISSING_CONTEXT_ERROR = "Couldn't find a schedule context."; - -export const ScheduleUpdateContext = React.createContext<{ - scheduleUpdate: (callback: () => void) => void; - flushUpdates: () => void; -}>({ - scheduleUpdate() { - throw new Error(MISSING_CONTEXT_ERROR); - }, - flushUpdates() { - throw new Error(MISSING_CONTEXT_ERROR); - }, -}); - -/** - * When screen config changes, we want to update the navigator in the same update phase. - * However, navigation state is in the root component and React won't let us update it from a child. - * This is a workaround for that, the scheduled update is stored in the ref without actually calling setState. - * It lets all subsequent updates access the latest state so it stays correct. - * Then we call setState during after the component updates. - */ -export function useScheduleUpdate(callback: () => void) { - const { scheduleUpdate, flushUpdates } = React.useContext( - ScheduleUpdateContext - ); - - scheduleUpdate(callback); - - React.useEffect(flushUpdates); -} diff --git a/packages/core/src/useSyncState.tsx b/packages/core/src/useSyncState.tsx index cdab94cc83..f085cce46c 100644 --- a/packages/core/src/useSyncState.tsx +++ b/packages/core/src/useSyncState.tsx @@ -1,74 +1,46 @@ import * as React from 'react'; -const UNINTIALIZED_STATE = {}; +const createStore = (getInitialState: () => T) => { + const listeners: (() => void)[] = []; -/** - * This is definitely not compatible with concurrent mode, but we don't have a solution for sync state yet. - */ -export function useSyncState(initialState?: (() => T) | T) { - const stateRef = React.useRef(UNINTIALIZED_STATE as any); - const isSchedulingRef = React.useRef(false); - const isMountedRef = React.useRef(true); + let state: T = getInitialState(); - React.useEffect(() => { - isMountedRef.current = true; + const getState = () => state; - return () => { - isMountedRef.current = false; - }; - }, []); - - if (stateRef.current === UNINTIALIZED_STATE) { - stateRef.current = - // @ts-expect-error: initialState is a function, but TypeScript doesn't think so - typeof initialState === 'function' ? initialState() : initialState; - } - - const [trackingState, setTrackingState] = React.useState(stateRef.current); - - const getState = React.useCallback(() => stateRef.current, []); + const setState = (newState: T) => { + state = newState; + listeners.forEach((listener) => listener()); + }; - const setState = React.useCallback((state: T) => { - if (state === stateRef.current || !isMountedRef.current) { - return; - } + const subscribe = (callback: () => void) => { + listeners.push(callback); - stateRef.current = state; - - if (!isSchedulingRef.current) { - setTrackingState(state); - } - }, []); - - const scheduleUpdate = React.useCallback((callback: () => void) => { - isSchedulingRef.current = true; - - try { - callback(); - } finally { - isSchedulingRef.current = false; - } - }, []); + return () => { + const index = listeners.indexOf(callback); - const flushUpdates = React.useCallback(() => { - if (!isMountedRef.current) { - return; - } + if (index > -1) { + listeners.splice(index, 1); + } + }; + }; - // Make sure that the tracking state is up-to-date. - // We call it unconditionally, but React should skip the update if state is unchanged. - setTrackingState(stateRef.current); - }, []); + return { + getState, + setState, + subscribe, + }; +}; - // If we're rendering and the tracking state is out of date, update it immediately - // This will make sure that our updates are applied as early as possible. - if (trackingState !== stateRef.current) { - setTrackingState(stateRef.current); - } +export function useSyncState(getInitialState: () => T) { + const store = React.useRef(createStore(getInitialState)).current; - const state = stateRef.current; + const state = React.useSyncExternalStore( + store.subscribe, + store.getState, + store.getState + ); React.useDebugValue(state); - return [state, getState, setState, scheduleUpdate, flushUpdates] as const; + return [state, store.getState, store.setState] as const; } diff --git a/packages/native/src/__tests__/ServerContainer.test.tsx b/packages/native/src/__tests__/ServerContainer.test.tsx index 01dcedd2c4..c8fe2f3d71 100644 --- a/packages/native/src/__tests__/ServerContainer.test.tsx +++ b/packages/native/src/__tests__/ServerContainer.test.tsx @@ -23,13 +23,19 @@ window.removeEventListener = () => {}; jest.mock('../useLinking', () => require('../useLinking.tsx')); // Since Jest is configured for React Native, the *.native.js file is imported -// But as we're testing server rendering, we want to use the web version -// So we mock it to point to the web version -jest.mock( - 'use-latest-callback/lib/useIsomorphicLayoutEffect', - // eslint-disable-next-line @typescript-eslint/no-var-requires - () => require('react').useEffect -); +// Causing the wrong useIsomorphicLayoutEffect to be imported +// It causes "Warning: useLayoutEffect does nothing on the server" +// So we explicitly silence it here +// This warning is being removed in React: https://github.com/facebook/react/pull/26395 +const error = console.error; + +jest.spyOn(console, 'error').mockImplementation((...args) => { + if (/Warning: useLayoutEffect does nothing on the server/m.test(args[0])) { + return; + } + + error(...args); +}); it('renders correct state with location', () => { const createStackNavigator = createNavigatorFactory((props: any) => {