diff --git a/packages/react-reconciler/src/ReactFiberExpirationTime.js b/packages/react-reconciler/src/ReactFiberExpirationTime.js index c93171032a926..7143173c9c162 100644 --- a/packages/react-reconciler/src/ReactFiberExpirationTime.js +++ b/packages/react-reconciler/src/ReactFiberExpirationTime.js @@ -29,8 +29,8 @@ export function expirationTimeToMs(expirationTime: ExpirationTime): number { return (expirationTime - MAGIC_NUMBER_OFFSET) * UNIT_SIZE; } -function round(num: number, precision: number): number { - return ((num / precision) | 0) * precision; +function ceiling(num: number, precision: number): number { + return (((num / precision) | 0) + 1) * precision; } export function computeExpirationBucket( @@ -40,7 +40,7 @@ export function computeExpirationBucket( ): ExpirationTime { return ( MAGIC_NUMBER_OFFSET + - round( + ceiling( currentTime - MAGIC_NUMBER_OFFSET + expirationInMs / UNIT_SIZE, bucketSizeMs / UNIT_SIZE, ) diff --git a/packages/react-reconciler/src/ReactFiberPendingPriority.js b/packages/react-reconciler/src/ReactFiberPendingPriority.js index e84957846e236..b34b2667139e0 100644 --- a/packages/react-reconciler/src/ReactFiberPendingPriority.js +++ b/packages/react-reconciler/src/ReactFiberPendingPriority.js @@ -167,22 +167,7 @@ export function markPingedPriorityLevel( if (latestSuspendedTime !== NoWork && latestSuspendedTime <= pingedTime) { const latestPingedTime = root.latestPingedTime; if (latestPingedTime === NoWork || latestPingedTime < pingedTime) { - // TODO: At one point, we decided we'd always work on the lowest priority - // suspended level. Part of the reasoning was to avoid displaying - // intermediate suspended states, e.g. if you click on two tabs in quick - // succession, only the final tab should render. But we later realized - // that the correct solution to this problem is in user space, e.g. by - // using a setState updater for the lower priority update that refers - // to the most recent high priority value. - // - // The only reason we track the lowest suspended level is so we don't have - // to track *every* suspended level. It's good enough to work on the last - // one. But in case of a ping, we know exactly what level we received, so - // we can go ahead and work on that one. - // - // Consider using the commented-out line instead: - // root.latestPingedTime = pingedTime; - root.latestPingedTime = latestSuspendedTime; + root.latestPingedTime = pingedTime; } } } diff --git a/packages/react-reconciler/src/ReactFiberUnwindWork.js b/packages/react-reconciler/src/ReactFiberUnwindWork.js index 571355a457889..72933f434aeab 100644 --- a/packages/react-reconciler/src/ReactFiberUnwindWork.js +++ b/packages/react-reconciler/src/ReactFiberUnwindWork.js @@ -49,7 +49,7 @@ import { enableSuspense, } from 'shared/ReactFeatureFlags'; -import {Sync, expirationTimeToMs} from './ReactFiberExpirationTime'; +import {Never, Sync, expirationTimeToMs} from './ReactFiberExpirationTime'; export default function( config: HostConfig, @@ -182,7 +182,10 @@ export default function( const expirationTimeMs = expirationTimeToMs(renderExpirationTime); const startTimeMs = expirationTimeMs - 5000; - const elapsedMs = currentTimeMs - startTimeMs; + let elapsedMs = currentTimeMs - startTimeMs; + if (elapsedMs < 0) { + elapsedMs = 0; + } const remainingTimeMs = expirationTimeMs - currentTimeMs; // Find the earliest timeout of all the timeouts in the ancestor path. @@ -221,7 +224,7 @@ export default function( // Compute the remaining time until the timeout. const msUntilTimeout = earliestTimeoutMs - elapsedMs; - if (msUntilTimeout > 0) { + if (renderExpirationTime === Never || msUntilTimeout > 0) { // There's still time remaining. suspendRoot(root, thenable, msUntilTimeout, renderExpirationTime); const onResolveOrReject = () => { diff --git a/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js index afc36509e74a3..e834ca5eda8e1 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js @@ -193,7 +193,7 @@ describe('ReactSuspense', () => { return ( - + ); @@ -204,8 +204,8 @@ describe('ReactSuspense', () => { expect(ReactNoop.getChildren()).toEqual([]); textResourceShouldFail = true; - ReactNoop.expire(100); - await advanceTimers(100); + ReactNoop.expire(1000); + await advanceTimers(1000); textResourceShouldFail = false; expect(ReactNoop.flush()).toEqual([ @@ -222,8 +222,8 @@ describe('ReactSuspense', () => { cache.invalidate(); expect(ReactNoop.flush()).toEqual(['Suspend! [Result]']); - ReactNoop.expire(100); - await advanceTimers(100); + ReactNoop.expire(1000); + await advanceTimers(1000); expect(ReactNoop.flush()).toEqual(['Promise resolved [Result]', 'Result']); expect(ReactNoop.getChildren()).toEqual([span('Result')]); }); @@ -248,9 +248,9 @@ describe('ReactSuspense', () => { const errorBoundary = React.createRef(); function App() { return ( - }> + }> - + ); @@ -260,14 +260,14 @@ describe('ReactSuspense', () => { expect(ReactNoop.flush()).toEqual(['Suspend! [Result]']); expect(ReactNoop.getChildren()).toEqual([]); - ReactNoop.expire(50); - await advanceTimers(50); + ReactNoop.expire(2000); + await advanceTimers(2000); expect(ReactNoop.flush()).toEqual(['Suspend! [Result]', 'Loading...']); expect(ReactNoop.getChildren()).toEqual([span('Loading...')]); textResourceShouldFail = true; - ReactNoop.expire(50); - await advanceTimers(50); + ReactNoop.expire(1000); + await advanceTimers(1000); textResourceShouldFail = false; expect(ReactNoop.flush()).toEqual([ @@ -283,9 +283,9 @@ describe('ReactSuspense', () => { errorBoundary.current.reset(); cache.invalidate(); - expect(ReactNoop.flush()).toEqual(['Suspend! [Result]', 'Loading...']); - ReactNoop.expire(100); - await advanceTimers(100); + expect(ReactNoop.flush()).toEqual(['Suspend! [Result]']); + ReactNoop.expire(3000); + await advanceTimers(3000); expect(ReactNoop.flush()).toEqual(['Promise resolved [Result]', 'Result']); expect(ReactNoop.getChildren()).toEqual([span('Result')]); }); @@ -495,8 +495,8 @@ describe('ReactSuspense', () => { // Expire the outer timeout, but don't expire the inner one. // We should see the outer loading placeholder. - ReactNoop.expire(1000); - await advanceTimers(1000); + ReactNoop.expire(1500); + await advanceTimers(1500); expect(ReactNoop.flush()).toEqual([ 'Sync', // Still suspended. @@ -600,8 +600,8 @@ describe('ReactSuspense', () => { it('expires early with a `timeout` option', async () => { ReactNoop.render( - }> - + }> + , @@ -619,8 +619,8 @@ describe('ReactSuspense', () => { // Advance both React's virtual time and Jest's timers by enough to trigger // the timeout, but not by enough to flush the promise or reach the true // expiration time. - ReactNoop.expire(120); - await advanceTimers(120); + ReactNoop.expire(2000); + await advanceTimers(2000); expect(ReactNoop.flush()).toEqual([ // Still suspended. 'Suspend! [Async]', @@ -763,7 +763,7 @@ describe('ReactSuspense', () => { {didTimeout => ( {didTimeout ? : null} @@ -776,8 +776,8 @@ describe('ReactSuspense', () => { expect(ReactNoop.flush()).toEqual(['Suspend! [Async]']); expect(ReactNoop.getChildren()).toEqual([]); - ReactNoop.expire(1000); - await advanceTimers(1000); + ReactNoop.expire(2000); + await advanceTimers(2000); expect(ReactNoop.flush()).toEqual([ 'Suspend! [Async]', 'Loading...', @@ -932,13 +932,13 @@ describe('ReactSuspense', () => { ReactNoop.flush(); expect(ReactNoop.getChildren()).toEqual([]); - await advanceTimers(999); - ReactNoop.expire(999); + await advanceTimers(800); + ReactNoop.expire(800); ReactNoop.flush(); expect(ReactNoop.getChildren()).toEqual([]); - await advanceTimers(1); - ReactNoop.expire(1); + await advanceTimers(1000); + ReactNoop.expire(1000); ReactNoop.flush(); expect(ReactNoop.getChildren()).toEqual([span('A')]); });