@samitouri / QOS-React / commits / a8f971b7a6

Switch to mount dispatcher after use() when needed (#26232)

When resuming a suspended render, there may be more Hooks to be called that weren't seen the previous time through. Make sure to switch to the mount dispatcher when calling use() if the next Hook call should be treated as a mount. Fixes #25964.

Sophie Alpert committed Feb 24, 2023 at 12:06 UTC a8f971b7a669a9a6321b9f3cea820f68b2e4ac6e

4 files changed +187 -34

packages/react-reconciler/src/ReactFiberHooks.js

+39 -31

@@ -658,10 +658,6 @@ export function replaySuspendedComponentWithHooks<Props, SecondArg>(

658 // only get reset when the component either completes (finishRenderingHooks)

659 // or unwinds (resetHooksOnUnwind).

660 if (__DEV__) {

661 - hookTypesDev =

662 - current !== null

663 - ? ((current._debugHookTypes: any): Array<HookType>)

664 - : null;

661 hookTypesUpdateIndexDev = -1;

662 // Used for hot reloading:

663 ignorePreviousDependencies =

@@ -696,8 +692,13 @@ function renderWithHooksAgain<Props, SecondArg>(

692 let numberOfReRenders: number = 0;

693 let children;

694 do {

699 - didScheduleRenderPhaseUpdateDuringThisPass = false;

695 + if (didScheduleRenderPhaseUpdateDuringThisPass) {

696 + // It's possible that a use() value depended on a state that was updated in

697 + // this rerender, so we need to watch for different thenables this time.

698 + thenableState = null;

699 + }

700 thenableIndexCounter = 0;

701 + didScheduleRenderPhaseUpdateDuringThisPass = false;

702

703 if (numberOfReRenders >= RE_RENDER_LIMIT) {

704 throw new Error(

@@ -841,8 +842,7 @@ function updateWorkInProgressHook(): Hook {

842 // This function is used both for updates and for re-renders triggered by a

843 // render phase update. It assumes there is either a current hook we can

844 // clone, or a work-in-progress hook from a previous render pass that we can

844 - // use as a base. When we reach the end of the base list, we must switch to

845 - // the dispatcher used for mounts.

845 + // use as a base.

846 let nextCurrentHook: null | Hook;

847 if (currentHook === null) {

848 const current = currentlyRenderingFiber.alternate;

@@ -876,16 +876,10 @@ function updateWorkInProgressHook(): Hook {

876 if (currentFiber === null) {

877 // This is the initial render. This branch is reached when the component

878 // suspends, resumes, then renders an additional hook.

879 - const newHook: Hook = {

880 - memoizedState: null,

881 -

882 - baseState: null,

883 - baseQueue: null,

884 - queue: null,

885 -

886 - next: null,

887 - };

888 - nextCurrentHook = newHook;

879 + // Should never be reached because we should switch to the mount dispatcher first.

880 + throw new Error(

881 + 'Update hook called on initial render. This is likely a bug in React. Please file an issue.',

882 + );

883 } else {

884 // This is an update. We should always have a current hook.

885 throw new Error('Rendered more hooks than during the previous render.');

@@ -951,7 +945,24 @@ function use<T>(usable: Usable<T>): T {

945 if (thenableState === null) {

946 thenableState = createThenableState();

947 }

954 - return trackUsedThenable(thenableState, thenable, index);

948 + const result = trackUsedThenable(thenableState, thenable, index);

949 + if (

950 + currentlyRenderingFiber.alternate === null &&

951 + (workInProgressHook === null

952 + ? currentlyRenderingFiber.memoizedState === null

953 + : workInProgressHook.next === null)

954 + ) {

955 + // Initial render, and either this is the first time the component is

956 + // called, or there were no Hooks called after this use() the previous

957 + // time (perhaps because it threw). Subsequent Hook calls should use the

958 + // mount dispatcher.

959 + if (__DEV__) {

960 + ReactCurrentDispatcher.current = HooksDispatcherOnMountInDEV;

961 + } else {

962 + ReactCurrentDispatcher.current = HooksDispatcherOnMount;

963 + }

964 + }

965 + return result;

966 } else if (

967 usable.$$typeof === REACT_CONTEXT_TYPE ||

968 usable.$$typeof === REACT_SERVER_CONTEXT_TYPE

@@ -1998,6 +2009,7 @@ function updateEffectImpl(

2009 const nextDeps = deps === undefined ? null : deps;

2010 let destroy = undefined;

2011

2012 + // currentHook is null when rerendering after a render phase state update.

2013 if (currentHook !== null) {

2014 const prevEffect = currentHook.memoizedState;

2015 destroy = prevEffect.destroy;

@@ -2250,12 +2262,10 @@ function updateCallback<T>(callback: T, deps: Array<mixed> | void | null): T {

2262 const hook = updateWorkInProgressHook();

2263 const nextDeps = deps === undefined ? null : deps;

2264 const prevState = hook.memoizedState;

2253 - if (prevState !== null) {

2254 - if (nextDeps !== null) {

2255 - const prevDeps: Array<mixed> | null = prevState[1];

2256 - if (areHookInputsEqual(nextDeps, prevDeps)) {

2257 - return prevState[0];

2258 - }

2265 + if (nextDeps !== null) {

2266 + const prevDeps: Array<mixed> | null = prevState[1];

2267 + if (areHookInputsEqual(nextDeps, prevDeps)) {

2268 + return prevState[0];

2269 }

2270 }

2271 hook.memoizedState = [callback, nextDeps];

@@ -2283,13 +2293,11 @@ function updateMemo<T>(

2293 const hook = updateWorkInProgressHook();

2294 const nextDeps = deps === undefined ? null : deps;

2295 const prevState = hook.memoizedState;

2286 - if (prevState !== null) {

2287 - // Assume these are defined. If they're not, areHookInputsEqual will warn.

2288 - if (nextDeps !== null) {

2289 - const prevDeps: Array<mixed> | null = prevState[1];

2290 - if (areHookInputsEqual(nextDeps, prevDeps)) {

2291 - return prevState[0];

2292 - }

2296 + // Assume these are defined. If they're not, areHookInputsEqual will warn.

2297 + if (nextDeps !== null) {

2298 + const prevDeps: Array<mixed> | null = prevState[1];

2299 + if (areHookInputsEqual(nextDeps, prevDeps)) {

2300 + return prevState[0];

2301 }

2302 }

2303 if (shouldDoubleInvokeUserFnsInHooksDEV) {

packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js

+1 -1

@@ -1076,7 +1076,7 @@ describe('ReactHooks', () => {

1076 expect(() => {

1077 ReactTestRenderer.create(<App />);

1078 }).toThrow(

1079 - 'Should have a queue. This is likely a bug in React. Please file an issue.',

1079 + 'Update hook called on initial render. This is likely a bug in React. Please file an issue.',

1080 );

1081 }).toErrorDev([

1082 'Do not call Hooks inside useEffect(...), useMemo(...), or other built-in Hooks',

packages/react-reconciler/src/__tests__/ReactThenable-test.js

+145 -1

@@ -5,6 +5,7 @@ let ReactNoop;

5 let Scheduler;

6 let act;

7 let use;

8 +let useDebugValue;

9 let useState;

10 let useMemo;

11 let Suspense;

@@ -21,6 +22,7 @@ describe('ReactThenable', () => {

22 Scheduler = require('scheduler');

23 act = require('jest-react').act;

24 use = React.use;

25 + useDebugValue = React.useDebugValue;

26 useState = React.useState;

27 useMemo = React.useMemo;

28 Suspense = React.Suspense;

@@ -794,7 +796,7 @@ describe('ReactThenable', () => {

796 expect(root).toMatchRenderedOutput('(empty)');

797 });

798

797 - test('when replaying a suspended component, reuses the hooks computed during the previous attempt', async () => {

799 + test('when replaying a suspended component, reuses the hooks computed during the previous attempt (Memo)', async () => {

800 function ExcitingText({text}) {

801 // This computes the uppercased version of some text. Pretend it's an

802 // expensive operation that we want to reuse.

@@ -846,6 +848,118 @@ describe('ReactThenable', () => {

848 ]);

849 });

850

851 + test('when replaying a suspended component, reuses the hooks computed during the previous attempt (State)', async () => {

852 + let _setFruit;

853 + let _setVegetable;

854 + function Kitchen() {

855 + const [fruit, setFruit] = useState('apple');

856 + _setFruit = setFruit;

857 + const usedFruit = use(getAsyncText(fruit));

858 + const [vegetable, setVegetable] = useState('carrot');

859 + _setVegetable = setVegetable;

860 + return <Text text={usedFruit + ' ' + vegetable} />;

861 + }

862 +

863 + // Initial render.

864 + const root = ReactNoop.createRoot();

865 + await act(async () => {

866 + startTransition(() => {

867 + root.render(<Kitchen />);

868 + });

869 + });

870 + expect(Scheduler).toHaveYielded(['Async text requested [apple]']);

871 + expect(root).toMatchRenderedOutput(null);

872 + await act(async () => {

873 + resolveTextRequests('apple');

874 + });

875 + expect(Scheduler).toHaveYielded([

876 + 'Async text requested [apple]',

877 + 'apple carrot',

878 + ]);

879 + expect(root).toMatchRenderedOutput('apple carrot');

880 +

881 + // Update the state variable after the use().

882 + await act(async () => {

883 + startTransition(() => {

884 + _setVegetable('dill');

885 + });

886 + });

887 + expect(Scheduler).toHaveYielded(['Async text requested [apple]']);

888 + expect(root).toMatchRenderedOutput('apple carrot');

889 + await act(async () => {

890 + resolveTextRequests('apple');

891 + });

892 + expect(Scheduler).toHaveYielded([

893 + 'Async text requested [apple]',

894 + 'apple dill',

895 + ]);

896 + expect(root).toMatchRenderedOutput('apple dill');

897 +

898 + // Update the state variable before the use(). The second state is maintained.

899 + await act(async () => {

900 + startTransition(() => {

901 + _setFruit('banana');

902 + });

903 + });

904 + expect(Scheduler).toHaveYielded(['Async text requested [banana]']);

905 + expect(root).toMatchRenderedOutput('apple dill');

906 + await act(async () => {

907 + resolveTextRequests('banana');

908 + });

909 + expect(Scheduler).toHaveYielded([

910 + 'Async text requested [banana]',

911 + 'banana dill',

912 + ]);

913 + expect(root).toMatchRenderedOutput('banana dill');

914 + });

915 +

916 + test('when replaying a suspended component, reuses the hooks computed during the previous attempt (DebugValue+State)', async () => {

917 + // Make sure we don't get a Hook mismatch warning on updates if there were non-stateful Hooks before the use().

918 + let _setLawyer;

919 + function Lexicon() {

920 + useDebugValue(123);

921 + const avocado = use(getAsyncText('aguacate'));

922 + const [lawyer, setLawyer] = useState('abogado');

923 + _setLawyer = setLawyer;

924 + return <Text text={avocado + ' ' + lawyer} />;

925 + }

926 +

927 + // Initial render.

928 + const root = ReactNoop.createRoot();

929 + await act(async () => {

930 + startTransition(() => {

931 + root.render(<Lexicon />);

932 + });

933 + });

934 + expect(Scheduler).toHaveYielded(['Async text requested [aguacate]']);

935 + expect(root).toMatchRenderedOutput(null);

936 + await act(async () => {

937 + resolveTextRequests('aguacate');

938 + });

939 + expect(Scheduler).toHaveYielded([

940 + 'Async text requested [aguacate]',

941 + 'aguacate abogado',

942 + ]);

943 + expect(root).toMatchRenderedOutput('aguacate abogado');

944 +

945 + // Now update the state.

946 + await act(async () => {

947 + startTransition(() => {

948 + _setLawyer('avocat');

949 + });

950 + });

951 + expect(Scheduler).toHaveYielded(['Async text requested [aguacate]']);

952 + expect(root).toMatchRenderedOutput('aguacate abogado');

953 + await act(async () => {

954 + resolveTextRequests('aguacate');

955 + });

956 + expect(Scheduler).toHaveYielded([

957 + 'Async text requested [aguacate]',

958 + 'aguacate avocat',

959 + ]);

960 + expect(root).toMatchRenderedOutput('aguacate avocat');

961 + });

962 +

963 // @gate enableUseHook

964 test(

965 'wrap an async function with useMemo to skip running the function ' +

@@ -1021,4 +1135,34 @@ describe('ReactThenable', () => {

1135 expect(Scheduler).toHaveYielded(['Async text requested [C]', 'C']);

1136 expect(root).toMatchRenderedOutput('ABC');

1137 });

1138 +

1139 + // @gate enableUseHook

1140 + test('use() combined with render phase updates', async () => {

1141 + function Async() {

1142 + const a = use(Promise.resolve('A'));

1143 + const [count, setCount] = useState(0);

1144 + if (count === 0) {

1145 + setCount(1);

1146 + }

1147 + const usedCount = use(Promise.resolve(count));

1148 + return <Text text={a + usedCount} />;

1149 + }

1150 +

1151 + function App() {

1152 + return (

1153 + <Suspense fallback={<Text text="Loading..." />}>

1154 + <Async />

1155 + </Suspense>

1156 + );

1157 + }

1158 +

1159 + const root = ReactNoop.createRoot();

1160 + await act(async () => {

1161 + startTransition(() => {

1162 + root.render(<App />);

1163 + });

1164 + });

1165 + expect(Scheduler).toHaveYielded(['A1']);

1166 + expect(root).toMatchRenderedOutput('A1');

1167 + });

1168 });

scripts/error-codes/codes.json

+2 -1

@@ -451,5 +451,6 @@

451 "463": "ReactDOMServer.renderToNodeStream(): The Node Stream API is not available in Bun. Use ReactDOMServer.renderToReadableStream() instead.",

452 "464": "ReactDOMServer.renderToStaticNodeStream(): The Node Stream API is not available in Bun. Use ReactDOMServer.renderToReadableStream() instead.",

453 "465": "enableFizzExternalRuntime without enableFloat is not supported. This should never appear in production, since it means you are using a misconfigured React bundle.",

454 - "466": "Trying to call a function from \"use server\" but the callServer option was not implemented in your router runtime."

454 + "466": "Trying to call a function from \"use server\" but the callServer option was not implemented in your router runtime.",

455 + "467": "Update hook called on initial render. This is likely a bug in React. Please file an issue."

456 }

\ No newline at end of file