From af4855ca7dae51534cd148a139049142eeecc5de Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Thu, 7 Nov 2019 15:33:07 -0800 Subject: [PATCH] Pass Suspense config to `startTransition` ...instead of `useTransition`. This avoids the problem of having to bind the config object to `startTransition`, invalidating downstream memoizations. --- .../react-reconciler/src/ReactFiberHooks.js | 54 +++++++------------ ...eactHooksWithNoopRenderer-test.internal.js | 47 +++++++++------- 2 files changed, 47 insertions(+), 54 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 14d70b5c5c459..cf4b5246d99a7 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -1172,50 +1172,34 @@ function updateDeferredValue( return prevValue; } +function startTransition(setPending, callback, config) { + setPending(true); + Scheduler.unstable_next(() => { + const previousConfig = ReactCurrentBatchConfig.suspense; + ReactCurrentBatchConfig.suspense = config === undefined ? null : config; + try { + setPending(false); + callback(); + } finally { + ReactCurrentBatchConfig.suspense = previousConfig; + } + }); +} + function mountTransition( config: SuspenseConfig | void | null, ): [(() => void) => void, boolean] { const [isPending, setPending] = mountState(false); - const startTransition = mountCallback( - callback => { - setPending(true); - Scheduler.unstable_next(() => { - const previousConfig = ReactCurrentBatchConfig.suspense; - ReactCurrentBatchConfig.suspense = config === undefined ? null : config; - try { - setPending(false); - callback(); - } finally { - ReactCurrentBatchConfig.suspense = previousConfig; - } - }); - }, - [config, isPending], - ); - return [startTransition, isPending]; + const startTransitionRef = mountRef(startTransition.bind(null, setPending)); + return [startTransitionRef.current, isPending]; } function updateTransition( config: SuspenseConfig | void | null, ): [(() => void) => void, boolean] { - const [isPending, setPending] = updateState(false); - const startTransition = updateCallback( - callback => { - setPending(true); - Scheduler.unstable_next(() => { - const previousConfig = ReactCurrentBatchConfig.suspense; - ReactCurrentBatchConfig.suspense = config === undefined ? null : config; - try { - setPending(false); - callback(); - } finally { - ReactCurrentBatchConfig.suspense = previousConfig; - } - }); - }, - [config, isPending], - ); - return [startTransition, isPending]; + const [isPending] = updateState(false); + const startTransitionRef = updateRef(); + return [startTransitionRef.current, isPending]; } function dispatchAction( diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js index c1954e36ed8c9..114bfc0971746 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js @@ -1977,16 +1977,18 @@ describe('ReactHooksWithNoopRenderer', () => { it.experimental( 'delays showing loading state until after timeout', async () => { + const SUSPENSE_CONFIG = { + timeoutMs: 1000, + }; + let transition; function App() { const [show, setShow] = useState(false); - const [startTransition, isPending] = useTransition({ - timeoutMs: 1000, - }); + const [startTransition, isPending] = useTransition(); transition = () => { startTransition(() => { setShow(true); - }); + }, SUSPENSE_CONFIG); }; return ( { it.experimental( 'delays showing loading state until after busyDelayMs + busyMinDurationMs', async () => { + const SUSPENSE_CONFIG = { + busyDelayMs: 1000, + busyMinDurationMs: 2000, + }; + let transition; function App() { const [show, setShow] = useState(false); - const [startTransition, isPending] = useTransition({ - busyDelayMs: 1000, - busyMinDurationMs: 2000, - }); + const [startTransition, isPending] = useTransition(); transition = () => { startTransition(() => { setShow(true); - }); + }, SUSPENSE_CONFIG); }; return ( { ); it.experimental('always returns the same startTransition', async () => { + const SUSPENSE_CONFIG = { + busyDelayMs: 1000, + busyMinDurationMs: 2000, + }; + let transition; function App() { const [step, setStep] = useState(0); - const [startTransition, isPending] = useTransition({ - busyDelayMs: 1000, - busyMinDurationMs: 2000, - }); + const [startTransition, isPending] = useTransition(); // Log whenever startTransition changes useEffect( () => { @@ -2130,7 +2136,7 @@ describe('ReactHooksWithNoopRenderer', () => { transition = () => { startTransition(() => { setStep(n => n + 1); - }); + }, SUSPENSE_CONFIG); }; return ( }> @@ -2175,12 +2181,12 @@ describe('ReactHooksWithNoopRenderer', () => { }); it.experimental( - 'can update suspense config (without changing startTransition)', + 'can pass different suspense configs per transition', async () => { let transition; function App({timeoutMs}) { const [step, setStep] = useState(0); - const [startTransition, isPending] = useTransition({timeoutMs}); + const [startTransition, isPending] = useTransition(); // Log whenever startTransition changes useEffect( () => { @@ -2189,9 +2195,12 @@ describe('ReactHooksWithNoopRenderer', () => { [startTransition], ); transition = () => { - startTransition(() => { - setStep(n => n + 1); - }); + startTransition( + () => { + setStep(n => n + 1); + }, + {timeoutMs}, + ); }; return ( }>