Skip to content

Commit 6542b82

Browse files
committed
fix: rendered more hooks issue
1 parent c260b38 commit 6542b82

File tree

2 files changed

+252
-4
lines changed

2 files changed

+252
-4
lines changed

packages/react-dom/src/__tests__/ReactDOMFizzShellHydration-test.js

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -656,4 +656,135 @@ describe('ReactDOMFizzShellHydration', () => {
656656
expect(container.innerHTML).toBe('Client');
657657
},
658658
);
659+
660+
it('handles conditional use with a cascading update and error boundaries', async () => {
661+
class ErrorBoundary extends React.Component {
662+
constructor(props) {
663+
super(props);
664+
this.state = {error: null};
665+
}
666+
667+
static getDerivedStateFromError(error) {
668+
return {error};
669+
}
670+
671+
componentDidCatch() {}
672+
673+
render() {
674+
if (this.state.error) {
675+
return 'Something went wrong: ' + this.state.error.message;
676+
}
677+
678+
return this.props.children;
679+
}
680+
}
681+
682+
function Bomb() {
683+
throw new Error('boom');
684+
}
685+
686+
function Updater({setPromise}) {
687+
const [state, setState] = React.useState(false);
688+
689+
React.useEffect(() => {
690+
// deleting this set state removes too many hooks error
691+
setState(true);
692+
// deleting this startTransition removes too many hooks error
693+
startTransition(() => {
694+
setPromise(Promise.resolve('resolved'));
695+
});
696+
}, [state]);
697+
698+
return null;
699+
}
700+
701+
const Context = React.createContext(null);
702+
703+
function Page() {
704+
const [promise, setPromise] = React.useState(null);
705+
Scheduler.log('use: ' + promise);
706+
/**
707+
* this part is tricky, I cannot say confidently the conditional `use` is required for the reproduction.
708+
* If we tried to run use(promise ?? cachedPromise) we wouldn't be able renderToString without a parent suspense boundary
709+
* but with a parent suspense the bug is no longer reproducible (with or without conditional use)
710+
* and without renderToString + hydration, the bug is no longer reproducible
711+
*/
712+
const value = promise ? React.use(promise) : promise;
713+
Scheduler.log('used: ' + value);
714+
715+
React.useMemo(() => {}, []);
716+
React.useCallback(() => {}, []);
717+
React.useEffect(() => {}, []);
718+
React.useLayoutEffect(() => {}, []);
719+
React.useRef(null);
720+
React.useImperativeHandle(null, () => {});
721+
React.useDebugValue(null);
722+
React.useContext(Context);
723+
const [state] = React.useState();
724+
React.useDeferredValue(state);
725+
React.useOptimistic(state, () => {});
726+
React.useTransition();
727+
React.useSyncExternalStore(
728+
() => () => {},
729+
() => null,
730+
() => null,
731+
);
732+
return (
733+
<>
734+
<Updater setPromise={setPromise} />
735+
<React.Suspense fallback="Loading...">
736+
<ErrorBoundary>
737+
<Bomb />
738+
</ErrorBoundary>
739+
</React.Suspense>
740+
hello world
741+
</>
742+
);
743+
}
744+
function App() {
745+
return <Page />;
746+
}
747+
748+
// Server render
749+
await serverAct(async () => {
750+
const {pipe} = ReactDOMFizzServer.renderToPipeableStream(<App />, {
751+
onError(error) {
752+
Scheduler.log('onError: ' + error.message);
753+
},
754+
});
755+
pipe(writable);
756+
});
757+
assertLog(['use: null', 'used: null', 'onError: boom']);
758+
759+
expect(container.textContent).toBe('Loading...hello world');
760+
761+
await clientAct(async () => {
762+
ReactDOMClient.hydrateRoot(container, <App />, {
763+
onCaughtError(error) {
764+
Scheduler.log('onCaughtError: ' + error.message);
765+
},
766+
onUncaughtError(error) {
767+
Scheduler.log('onUncaughtError: ' + error.message);
768+
},
769+
onRecoverableError(error) {
770+
Scheduler.log('onRecoverableError: ' + error.message);
771+
if (error.cause) {
772+
Scheduler.log('Cause: ' + error.cause.message);
773+
}
774+
},
775+
});
776+
});
777+
assertLog([
778+
'use: null',
779+
'used: null',
780+
'use: [object Promise]',
781+
'onCaughtError: boom',
782+
'use: [object Promise]',
783+
'use: [object Promise]',
784+
'used: resolved',
785+
]);
786+
787+
// Should've rendered something. The error was handled by the error boundary.
788+
expect(container.textContent).toBe('Something went wrong: boomhello world');
789+
});
659790
});

packages/react-reconciler/src/ReactFiberHooks.js

Lines changed: 121 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -281,6 +281,9 @@ let localIdCounter: number = 0;
281281
let thenableIndexCounter: number = 0;
282282
let thenableState: ThenableState | null = null;
283283

284+
// Track whether we've switched dispatchers due to conditional use
285+
let hasDispatcherSwitchedDueToUse: boolean = false;
286+
284287
// Used for ids that are generated completely client-side (i.e. not during
285288
// hydration). This counter is global, so client ids are not stable across
286289
// render attempts.
@@ -624,6 +627,9 @@ export function renderWithHooks<Props, SecondArg>(
624627

625628
finishRenderingHooks(current, workInProgress, Component);
626629

630+
// Reset dispatcher switch flag for the next render
631+
hasDispatcherSwitchedDueToUse = false;
632+
627633
return children;
628634
}
629635

@@ -931,6 +937,9 @@ export function resetHooksAfterThrow(): void {
931937
// We can assume the previous dispatcher is always this one, since we set it
932938
// at the beginning of the render phase and there's no re-entrance.
933939
ReactSharedInternals.H = ContextOnlyDispatcher;
940+
941+
// Reset dispatcher switch flag
942+
hasDispatcherSwitchedDueToUse = false;
934943
}
935944

936945
export function resetHooksOnUnwind(workInProgress: Fiber): void {
@@ -1037,6 +1046,32 @@ function updateWorkInProgressHook(): Hook {
10371046
'Update hook called on initial render. This is likely a bug in React. Please file an issue.',
10381047
);
10391048
} else {
1049+
// Check if we're in a conditional use scenario
1050+
if (hasDispatcherSwitchedDueToUse) {
1051+
// We're in a situation where conditional use has caused hook count mismatch.
1052+
// Create a new hook and mark that we should treat the next hook as a mount.
1053+
const newHook: Hook = {
1054+
memoizedState: null,
1055+
baseState: null,
1056+
baseQueue: null,
1057+
queue: null,
1058+
next: null,
1059+
};
1060+
1061+
if (workInProgressHook === null) {
1062+
// This is the first hook in the list.
1063+
currentlyRenderingFiber.memoizedState = workInProgressHook =
1064+
newHook;
1065+
} else {
1066+
// Append to the end of the list.
1067+
workInProgressHook = workInProgressHook.next = newHook;
1068+
}
1069+
1070+
// Don't throw error - let the hook continue
1071+
// The specific hook implementation will handle initialization
1072+
return workInProgressHook;
1073+
}
1074+
10401075
// This is an update. We should always have a current hook.
10411076
throw new Error('Rendered more hooks than during the previous render.');
10421077
}
@@ -1130,11 +1165,15 @@ function useThenable<T>(thenable: Thenable<T>): T {
11301165
const currentFiber = workInProgressFiber.alternate;
11311166
if (__DEV__) {
11321167
if (currentFiber !== null && currentFiber.memoizedState !== null) {
1168+
hasDispatcherSwitchedDueToUse = true;
11331169
ReactSharedInternals.H = HooksDispatcherOnUpdateInDEV;
11341170
} else {
11351171
ReactSharedInternals.H = HooksDispatcherOnMountInDEV;
11361172
}
11371173
} else {
1174+
if (currentFiber !== null && currentFiber.memoizedState !== null) {
1175+
hasDispatcherSwitchedDueToUse = true;
1176+
}
11381177
ReactSharedInternals.H =
11391178
currentFiber === null || currentFiber.memoizedState === null
11401179
? HooksDispatcherOnMount
@@ -1304,10 +1343,20 @@ function updateReducerImpl<S, A>(
13041343
const queue = hook.queue;
13051344

13061345
if (queue === null) {
1307-
throw new Error(
1308-
'Should have a queue. You are likely calling Hooks conditionally, ' +
1309-
'which is not allowed. (https://react.dev/link/invalid-hook-call)',
1310-
);
1346+
// Check if this is a conditional use scenario
1347+
if (handleConditionalUseInit(hook)) {
1348+
// For conditional use, delegate to mount behavior with default initial value
1349+
const initialState =
1350+
reducer === basicStateReducer ? false : ((undefined: any): S);
1351+
return callWithConditionalUseFlag(() =>
1352+
mountReducer(reducer, initialState, undefined),
1353+
);
1354+
} else {
1355+
throw new Error(
1356+
'Should have a queue. You are likely calling Hooks conditionally, ' +
1357+
'which is not allowed. (https://react.dev/link/invalid-hook-call)',
1358+
);
1359+
}
13111360
}
13121361

13131362
queue.lastRenderedReducer = reducer;
@@ -1761,6 +1810,14 @@ function updateSyncExternalStore<T>(
17611810
}
17621811
const inst = hook.queue;
17631812

1813+
// Handle conditional use scenario where hook was newly created
1814+
if (handleConditionalUseInit(hook) && inst === null) {
1815+
// Delegate to mount behavior
1816+
return callWithConditionalUseFlag(() =>
1817+
mountSyncExternalStore(subscribe, getSnapshot, getServerSnapshot),
1818+
);
1819+
}
1820+
17641821
updateEffect(subscribeToStore.bind(null, fiber, inst, subscribe), [
17651822
subscribe,
17661823
]);
@@ -2633,6 +2690,15 @@ function updateEffectImpl(
26332690
): void {
26342691
const hook = updateWorkInProgressHook();
26352692
const nextDeps = deps === undefined ? null : deps;
2693+
2694+
// Handle conditional use scenario where hook was newly created
2695+
if (handleConditionalUseInit(hook)) {
2696+
// Delegate to mount behavior
2697+
return callWithConditionalUseFlag(() =>
2698+
mountEffectImpl(fiberFlags, hookFlags, create, deps),
2699+
);
2700+
}
2701+
26362702
const effect: Effect = hook.memoizedState;
26372703
const inst = effect.inst;
26382704

@@ -2900,6 +2966,13 @@ function updateCallback<T>(callback: T, deps: Array<mixed> | void | null): T {
29002966
const hook = updateWorkInProgressHook();
29012967
const nextDeps = deps === undefined ? null : deps;
29022968
const prevState = hook.memoizedState;
2969+
2970+
// Handle conditional use scenario where hook was newly created
2971+
if (handleConditionalUseInit(hook)) {
2972+
// Delegate to mount behavior
2973+
return callWithConditionalUseFlag(() => mountCallback(callback, deps));
2974+
}
2975+
29032976
if (nextDeps !== null) {
29042977
const prevDeps: Array<mixed> | null = prevState[1];
29052978
if (areHookInputsEqual(nextDeps, prevDeps)) {
@@ -2929,13 +3002,36 @@ function mountMemo<T>(
29293002
return nextValue;
29303003
}
29313004

3005+
// Helper function to handle conditional use initialization
3006+
function handleConditionalUseInit(hook: Hook): boolean {
3007+
return hasDispatcherSwitchedDueToUse && hook.memoizedState === null;
3008+
}
3009+
3010+
// Wrapper to safely call mount functions during conditional use
3011+
function callWithConditionalUseFlag<T>(fn: () => T): T {
3012+
const prevFlag = hasDispatcherSwitchedDueToUse;
3013+
hasDispatcherSwitchedDueToUse = false;
3014+
try {
3015+
return fn();
3016+
} finally {
3017+
hasDispatcherSwitchedDueToUse = prevFlag;
3018+
}
3019+
}
3020+
29323021
function updateMemo<T>(
29333022
nextCreate: () => T,
29343023
deps: Array<mixed> | void | null,
29353024
): T {
29363025
const hook = updateWorkInProgressHook();
29373026
const nextDeps = deps === undefined ? null : deps;
29383027
const prevState = hook.memoizedState;
3028+
3029+
// Handle conditional use scenario where hook was newly created
3030+
if (handleConditionalUseInit(hook)) {
3031+
// Delegate to mount behavior
3032+
return callWithConditionalUseFlag(() => mountMemo(nextCreate, deps));
3033+
}
3034+
29393035
// Assume these are defined. If they're not, areHookInputsEqual will warn.
29403036
if (nextDeps !== null) {
29413037
const prevDeps: Array<mixed> | null = prevState[1];
@@ -3401,6 +3497,27 @@ function updateTransition(): [
34013497
const [booleanOrThenable] = updateState(false);
34023498
const hook = updateWorkInProgressHook();
34033499
const start = hook.memoizedState;
3500+
3501+
// Handle conditional use scenario
3502+
if (handleConditionalUseInit(hook)) {
3503+
// Delegate to mount behavior
3504+
return callWithConditionalUseFlag(() => mountTransition());
3505+
}
3506+
3507+
// Check if start is null for other error conditions
3508+
if (start === null) {
3509+
const stateHook = currentlyRenderingFiber.memoizedState;
3510+
const startFn = startTransition.bind(
3511+
null,
3512+
currentlyRenderingFiber,
3513+
stateHook.queue,
3514+
true,
3515+
false,
3516+
);
3517+
hook.memoizedState = startFn;
3518+
return [false, startFn];
3519+
}
3520+
34043521
const isPending =
34053522
typeof booleanOrThenable === 'boolean'
34063523
? booleanOrThenable

0 commit comments

Comments
 (0)