diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzShellHydration-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzShellHydration-test.js index d21059ba6cf..2cd05dd4a35 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzShellHydration-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzShellHydration-test.js @@ -656,4 +656,117 @@ describe('ReactDOMFizzShellHydration', () => { expect(container.innerHTML).toBe('Client'); }, ); + + it('handles conditional use with a cascading update and error boundaries', async () => { + class ErrorBoundary extends React.Component { + constructor(props) { + super(props); + this.state = {error: null}; + } + + static getDerivedStateFromError(error) { + return {error}; + } + + componentDidCatch() {} + + render() { + if (this.state.error) { + return 'Something went wrong: ' + this.state.error.message; + } + + return this.props.children; + } + } + + function Bomb() { + throw new Error('boom'); + } + + function Updater({setPromise}) { + const [state, setState] = React.useState(false); + + React.useEffect(() => { + // deleting this set state removes too many hooks error + setState(true); + // deleting this startTransition removes too many hooks error + startTransition(() => { + setPromise(Promise.resolve('resolved')); + }); + }, [state]); + + return null; + } + + function Page() { + const [promise, setPromise] = React.useState(null); + Scheduler.log('use: ' + promise); + /** + * this part is tricky, I cannot say confidently the conditional `use` is required for the reproduction. + * If we tried to run use(promise ?? cachedPromise) we wouldn't be able renderToString without a parent suspense boundary + * but with a parent suspense the bug is no longer reproducible (with or without conditional use) + * and without renderToString + hydration, the bug is no longer reproducible + */ + const value = promise ? React.use(promise) : promise; + Scheduler.log('used: ' + value); + + React.useMemo(() => {}, []); // to trigger too many hooks error + return ( + <> + + + + + + + hello world + + ); + } + function App() { + return ; + } + + // Server render + await serverAct(async () => { + const {pipe} = ReactDOMFizzServer.renderToPipeableStream(, { + onError(error) { + Scheduler.log('onError: ' + error.message); + }, + }); + pipe(writable); + }); + assertLog(['use: null', 'used: null', 'onError: boom']); + + expect(container.textContent).toBe('Loading...hello world'); + + await clientAct(async () => { + ReactDOMClient.hydrateRoot(container, , { + onCaughtError(error) { + Scheduler.log('onCaughtError: ' + error.message); + }, + onUncaughtError(error) { + Scheduler.log('onUncaughtError: ' + error.message); + }, + onRecoverableError(error) { + Scheduler.log('onRecoverableError: ' + error.message); + if (error.cause) { + Scheduler.log('Cause: ' + error.cause.message); + } + }, + }); + }); + assertLog([ + 'use: null', + 'used: null', + 'use: [object Promise]', + 'onCaughtError: boom', + 'use: [object Promise]', + 'use: [object Promise]', + 'used: resolved', + ]); + + // Should've rendered something. The error was handled by the error boundary. + expect(container.textContent).toBe('Something went wrong: boomhello world'); + }); }); diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 63332124e2f..ac393ec4907 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -281,6 +281,9 @@ let localIdCounter: number = 0; let thenableIndexCounter: number = 0; let thenableState: ThenableState | null = null; +// Track whether we've switched dispatchers due to conditional use +let hasDispatcherSwitchedDueToUse: boolean = false; + // Used for ids that are generated completely client-side (i.e. not during // hydration). This counter is global, so client ids are not stable across // render attempts. @@ -624,6 +627,9 @@ export function renderWithHooks( finishRenderingHooks(current, workInProgress, Component); + // Reset dispatcher switch flag for the next render + hasDispatcherSwitchedDueToUse = false; + return children; } @@ -931,6 +937,9 @@ export function resetHooksAfterThrow(): void { // We can assume the previous dispatcher is always this one, since we set it // at the beginning of the render phase and there's no re-entrance. ReactSharedInternals.H = ContextOnlyDispatcher; + + // Reset dispatcher switch flag + hasDispatcherSwitchedDueToUse = false; } export function resetHooksOnUnwind(workInProgress: Fiber): void { @@ -1037,6 +1046,32 @@ function updateWorkInProgressHook(): Hook { 'Update hook called on initial render. This is likely a bug in React. Please file an issue.', ); } else { + // Check if we're in a conditional use scenario + if (hasDispatcherSwitchedDueToUse) { + // We're in a situation where conditional use has caused hook count mismatch. + // Create a new hook and mark that we should treat the next hook as a mount. + const newHook: Hook = { + memoizedState: null, + baseState: null, + baseQueue: null, + queue: null, + next: null, + }; + + if (workInProgressHook === null) { + // This is the first hook in the list. + currentlyRenderingFiber.memoizedState = workInProgressHook = + newHook; + } else { + // Append to the end of the list. + workInProgressHook = workInProgressHook.next = newHook; + } + + // Don't throw error - let the hook continue + // The specific hook implementation will handle initialization + return workInProgressHook; + } + // This is an update. We should always have a current hook. throw new Error('Rendered more hooks than during the previous render.'); } @@ -1130,11 +1165,15 @@ function useThenable(thenable: Thenable): T { const currentFiber = workInProgressFiber.alternate; if (__DEV__) { if (currentFiber !== null && currentFiber.memoizedState !== null) { + hasDispatcherSwitchedDueToUse = true; ReactSharedInternals.H = HooksDispatcherOnUpdateInDEV; } else { ReactSharedInternals.H = HooksDispatcherOnMountInDEV; } } else { + if (currentFiber !== null && currentFiber.memoizedState !== null) { + hasDispatcherSwitchedDueToUse = true; + } ReactSharedInternals.H = currentFiber === null || currentFiber.memoizedState === null ? HooksDispatcherOnMount @@ -1301,13 +1340,36 @@ function updateReducerImpl( current: Hook, reducer: (S, A) => S, ): [S, Dispatch] { - const queue = hook.queue; + let queue = hook.queue; if (queue === null) { - throw new Error( - 'Should have a queue. You are likely calling Hooks conditionally, ' + - 'which is not allowed. (https://react.dev/link/invalid-hook-call)', - ); + // Check if this is a conditional use scenario + if (hasDispatcherSwitchedDueToUse && hook.memoizedState === null) { + // Initialize the hook for conditional use case + // For conditional use, we don't have the initial state, so we use the current state + // or a default value. This matches what would happen during mount. + const initialState = ((undefined: any): S); + hook.memoizedState = hook.baseState = initialState; + const newQueue: UpdateQueue = { + pending: null, + lanes: NoLanes, + dispatch: null, + lastRenderedReducer: reducer, + lastRenderedState: initialState, + }; + queue = hook.queue = newQueue; + const dispatch: Dispatch = (dispatchReducerAction.bind( + null, + currentlyRenderingFiber, + newQueue, + ): any); + newQueue.dispatch = dispatch; + } else { + throw new Error( + 'Should have a queue. You are likely calling Hooks conditionally, ' + + 'which is not allowed. (https://react.dev/link/invalid-hook-call)', + ); + } } queue.lastRenderedReducer = reducer; @@ -2900,6 +2962,14 @@ function updateCallback(callback: T, deps: Array | void | null): T { const hook = updateWorkInProgressHook(); const nextDeps = deps === undefined ? null : deps; const prevState = hook.memoizedState; + + // Handle conditional use scenario where hook was newly created + if (hasDispatcherSwitchedDueToUse && prevState === null) { + // This hook was created due to conditional use - initialize it properly + hook.memoizedState = [callback, nextDeps]; + return callback; + } + if (nextDeps !== null) { const prevDeps: Array | null = prevState[1]; if (areHookInputsEqual(nextDeps, prevDeps)) { @@ -2936,6 +3006,23 @@ function updateMemo( const hook = updateWorkInProgressHook(); const nextDeps = deps === undefined ? null : deps; const prevState = hook.memoizedState; + + // Handle conditional use scenario where hook was newly created + if (hasDispatcherSwitchedDueToUse && prevState === null) { + // This hook was created due to conditional use - initialize it properly + const nextValue = nextCreate(); + if (shouldDoubleInvokeUserFnsInHooksDEV) { + setIsStrictModeForDevtools(true); + try { + nextCreate(); + } finally { + setIsStrictModeForDevtools(false); + } + } + hook.memoizedState = [nextValue, nextDeps]; + return nextValue; + } + // Assume these are defined. If they're not, areHookInputsEqual will warn. if (nextDeps !== null) { const prevDeps: Array | null = prevState[1];