From f215727b85194caf25240a215f9a366cc5e1a1f5 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Mon, 26 Feb 2018 19:25:10 -0800 Subject: [PATCH] Revert "Revert soft expiration" This reverts commit 6e00daa5077d6ed73312cbb7d12634c1b4acbf89. --- packages/react-reconciler/src/ReactFiber.js | 5 + .../src/ReactFiberBeginWork.js | 48 +++ .../src/ReactFiberCommitWork.js | 7 + .../src/ReactFiberCompleteWork.js | 3 + .../src/ReactFiberUpdateQueue.js | 7 +- .../src/__tests__/ReactSuspense-test.js | 348 +++++++++++++++++- packages/react/src/React.js | 2 + packages/react/src/ReactElementValidator.js | 2 + packages/shared/ReactSymbols.js | 5 +- packages/shared/ReactTypeOfWork.js | 1 + 10 files changed, 422 insertions(+), 6 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiber.js b/packages/react-reconciler/src/ReactFiber.js index b4538a79c36db..a69338740bcd1 100644 --- a/packages/react-reconciler/src/ReactFiber.js +++ b/packages/react-reconciler/src/ReactFiber.js @@ -29,6 +29,7 @@ import { Mode, ContextProvider, ContextConsumer, + LoadingComponent, TimeoutComponent, } from 'shared/ReactTypeOfWork'; import getComponentName from 'shared/getComponentName'; @@ -43,6 +44,7 @@ import { REACT_PROVIDER_TYPE, REACT_CONTEXT_TYPE, REACT_ASYNC_MODE_TYPE, + REACT_LOADING_TYPE, REACT_TIMEOUT_TYPE, } from 'shared/ReactSymbols'; @@ -349,6 +351,9 @@ export function createFiberFromElement( case REACT_RETURN_TYPE: fiberTag = ReturnComponent; break; + case REACT_LOADING_TYPE: + fiberTag = LoadingComponent; + break; case REACT_TIMEOUT_TYPE: fiberTag = TimeoutComponent; break; diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index 963a06a4ea61f..e4a82257e62c7 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -30,6 +30,7 @@ import { Mode, ContextProvider, ContextConsumer, + LoadingComponent, TimeoutComponent, } from 'shared/ReactTypeOfWork'; import { @@ -728,6 +729,47 @@ export default function( return workInProgress.stateNode; } + function updateLoadingComponent( + current, + workInProgress, + renderExpirationTime, + ) { + const nextProps = workInProgress.pendingProps; + const prevProps = workInProgress.memoizedProps; + + let nextState = workInProgress.memoizedState; + if (nextState === null) { + nextState = workInProgress.memoizedState = false; + } + const prevState = current === null ? nextState : current.memoizedState; + + const updateQueue = workInProgress.updateQueue; + if (updateQueue !== null) { + nextState = workInProgress.memoizedState = processUpdateQueue( + current, + workInProgress, + updateQueue, + null, + nextProps, + renderExpirationTime, + ); + } + + const isLoading = nextState; + if (hasLegacyContextChanged()) { + // Normally we can bail out on props equality but if context has changed + // we don't do the bailout and we have to reuse existing props instead. + } else if (prevProps === nextProps && prevState === nextState) { + return bailoutOnAlreadyFinishedWork(current, workInProgress); + } + + const render = nextProps.children; + const nextChildren = render(isLoading); + workInProgress.memoizedProps = nextProps; + reconcileChildren(current, workInProgress, nextChildren); + return workInProgress.child; + } + function updateTimeoutComponent( current, workInProgress, @@ -1155,6 +1197,12 @@ export default function( // A return component is just a placeholder, we can just run through the // next one immediately. return null; + case LoadingComponent: + return updateLoadingComponent( + current, + workInProgress, + renderExpirationTime, + ); case TimeoutComponent: return updateTimeoutComponent( current, diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index 54031ea40e518..a327977f58697 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -25,6 +25,7 @@ import { HostText, HostPortal, CallComponent, + LoadingComponent, TimeoutComponent, } from 'shared/ReactTypeOfWork'; import ReactErrorUtils from 'shared/ReactErrorUtils'; @@ -244,6 +245,9 @@ export default function( // We have no life-cycles associated with portals. return; } + case LoadingComponent: { + return; + } case TimeoutComponent: { const updateQueue = finishedWork.updateQueue; if (updateQueue !== null) { @@ -814,6 +818,9 @@ export default function( case HostRoot: { return; } + case LoadingComponent: { + return; + } case TimeoutComponent: { return; } diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.js b/packages/react-reconciler/src/ReactFiberCompleteWork.js index 21aea96615d7e..1e6d0f8e643c3 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.js @@ -34,6 +34,7 @@ import { ContextConsumer, Fragment, Mode, + LoadingComponent, TimeoutComponent, } from 'shared/ReactTypeOfWork'; import { @@ -606,6 +607,8 @@ export default function( case ReturnComponent: // Does nothing. return null; + case LoadingComponent: + return null; case TimeoutComponent: if (workInProgress.effectTag & DidCapture) { workInProgress.effectTag |= Update; diff --git a/packages/react-reconciler/src/ReactFiberUpdateQueue.js b/packages/react-reconciler/src/ReactFiberUpdateQueue.js index df66807dce24a..86677d0696042 100644 --- a/packages/react-reconciler/src/ReactFiberUpdateQueue.js +++ b/packages/react-reconciler/src/ReactFiberUpdateQueue.js @@ -16,7 +16,11 @@ import { debugRenderPhaseSideEffectsForStrictMode, } from 'shared/ReactFeatureFlags'; import {Callback as CallbackEffect} from 'shared/ReactTypeOfSideEffect'; -import {ClassComponent, HostRoot} from 'shared/ReactTypeOfWork'; +import { + ClassComponent, + HostRoot, + LoadingComponent, +} from 'shared/ReactTypeOfWork'; import invariant from 'fbjs/lib/invariant'; import warning from 'fbjs/lib/warning'; import {StrictMode} from './ReactTypeOfMode'; @@ -194,6 +198,7 @@ export function getUpdateExpirationTime(fiber: Fiber): ExpirationTime { switch (fiber.tag) { case HostRoot: case ClassComponent: + case LoadingComponent: const updateQueue = fiber.updateQueue; if (updateQueue === null) { return NoWork; diff --git a/packages/react-reconciler/src/__tests__/ReactSuspense-test.js b/packages/react-reconciler/src/__tests__/ReactSuspense-test.js index 1fbaac197b07a..ae8c86813ab50 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspense-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspense-test.js @@ -2,6 +2,7 @@ let React; let Fragment; let ReactNoop; let SimpleCacheProvider; +let Loading; let Timeout; let cache; @@ -14,6 +15,7 @@ describe('ReactSuspense', () => { Fragment = React.Fragment; ReactNoop = require('react-noop-renderer'); SimpleCacheProvider = require('simple-cache-provider'); + Loading = React.Loading; Timeout = React.Timeout; cache = SimpleCacheProvider.createCache(); @@ -27,10 +29,10 @@ describe('ReactSuspense', () => { }, ([text, ms]) => text); }); - // function div(...children) { - // children = children.map(c => (typeof c === 'string' ? {text: c} : c)); - // return {type: 'div', children, prop: undefined}; - // } + function div(...children) { + children = children.map(c => (typeof c === 'string' ? {text: c} : c)); + return {type: 'div', children, prop: undefined}; + } function span(prop) { return {type: 'span', children: [], prop}; @@ -151,6 +153,344 @@ describe('ReactSuspense', () => { ]); }); + it('can render an alternate view at a higher priority', async () => { + function App(props) { + return ( + + {isLoading => ( + + {isLoading ? : null} + + + + {props.step >= 1 ? : null} + + )} + + ); + } + + ReactNoop.render(); + expect(ReactNoop.flush()).toEqual(['A', 'B', 'C']); + expect(ReactNoop.getChildren()).toEqual([span('A'), span('B'), span('C')]); + + ReactNoop.render(); + expect(ReactNoop.flush()).toEqual([ + 'A', + 'B', + 'C', + // D suspends, which triggers the loading state. + 'Suspend! [D]', + 'Loading...', + 'A', + 'B', + 'C', + ]); + expect(ReactNoop.getChildren()).toEqual([ + span('Loading...'), + span('A'), + span('B'), + span('C'), + ]); + + // Wait for data to resolve + await advanceTimers(100); + expect(ReactNoop.flush()).toEqual([ + 'Promise resolved [D]', + 'A', + 'B', + 'C', + 'D', + ]); + expect(ReactNoop.getChildren()).toEqual([ + span('A'), + span('B'), + span('C'), + span('D'), + ]); + }); + + it('can suspend inside a boundary', async () => { + function App(props) { + return ( + + {isLoading => { + if (isLoading) { + return ; + } + return props.step > 0 ? ( + + ) : ( + + ); + }} + + ); + } + + // Initial mount + ReactNoop.render(); + expect(ReactNoop.flush()).toEqual(['Initial text']); + expect(ReactNoop.getChildren()).toEqual([span('Initial text')]); + + ReactNoop.render(); + expect(ReactNoop.flush()).toEqual([ + 'Suspend! [Final result]', + 'Suspend! [Loading...]', + ]); + expect(ReactNoop.getChildren()).toEqual([span('Initial text')]); + + // Unblock the "Loading..." view + await advanceTimers(50); + + expect(ReactNoop.flush()).toEqual([ + // Renders the loading view, + 'Promise resolved [Loading...]', + 'Loading...', + ]); + expect(ReactNoop.getChildren()).toEqual([span('Loading...')]); + + // Unblock the rest. + await advanceTimers(50); + + // Now we can render the final result. + expect(ReactNoop.flush()).toEqual([ + 'Promise resolved [Final result]', + 'Final result', + ]); + expect(ReactNoop.getChildren()).toEqual([span('Final result')]); + }); + + it('nested boundaries do not capture if an outer boundary is suspended', async () => { + function App(props) { + return ( + + {() => ( + + {() => + props.text ? ( + + ) : ( + + ) + } + + )} + + ); + } + ReactNoop.render(); + expect(ReactNoop.flush()).toEqual(['Initial text']); + expect(ReactNoop.getChildren()).toEqual([span('Initial text')]); + + ReactNoop.render(); + expect(ReactNoop.flush()).toEqual(['Suspend! [A]', 'Initial text']); + expect(ReactNoop.getChildren()).toEqual([span('Initial text')]); + + // Move B into a later expiration bucket + ReactNoop.expire(2000); + ReactNoop.render(); + expect(ReactNoop.flush()).toEqual([ + // B is suspended + 'Suspend! [B]', + // It doesn't bother trying to render a loading state because it + // would be based on A, which we already know is suspended. + ]); + expect(ReactNoop.getChildren()).toEqual([span('Initial text')]); + + // Unblock + await advanceTimers(0); + expect(ReactNoop.flush()).toEqual([ + 'Promise resolved [A]', + 'Promise resolved [B]', + 'B', + ]); + expect(ReactNoop.getChildren()).toEqual([span('B')]); + }); + + it('can suspend, resume, then resume again in a later update, with correct bubbling', async () => { + function App(props) { + return ( + + {isLoading => ( + + {isLoading ? : null} + + + )} + + ); + } + + ReactNoop.render(); + ReactNoop.flush(); + await advanceTimers(0); + ReactNoop.flush(); + expect(ReactNoop.getChildren()).toEqual([span('Initial text')]); + + ReactNoop.render(); + ReactNoop.flush(); + expect(ReactNoop.getChildren()).toEqual([ + span('Loading...'), + span('Initial text'), + ]); + await advanceTimers(0); + ReactNoop.flush(); + expect(ReactNoop.getChildren()).toEqual([span('Update')]); + + ReactNoop.render(); + ReactNoop.flush(); + expect(ReactNoop.getChildren()).toEqual([ + span('Loading...'), + span('Update'), + ]); + await advanceTimers(0); + ReactNoop.flush(); + expect(ReactNoop.getChildren()).toEqual([span('Another update')]); + }); + + it('bubbles to next boundary if it suspends', async () => { + function App(props) { + return ( + + {isLoadingOuter => ( + + {isLoadingOuter ? : null} + + {isLoadingInner => ( +
+ {isLoadingInner ? ( + + ) : null} + {props.step > 0 ? ( + + ) : ( + + )} +
+ )} + + + )} + + ); + } + + ReactNoop.render(); + ReactNoop.flush(); + expect(ReactNoop.getChildren()).toEqual([div(span('Initial text'))]); + + // Update to display "Final result" + ReactNoop.render(); + expect(ReactNoop.flush()).toEqual([ + // "Final result" suspends. + 'Suspend! [Final result]', + // The inner boundary renders a loading view. The loading view also suspends. + 'Suspend! [Loading (inner)...]', + // (Continues rendering siblings even though something suspended) + 'Initial text', + // Bubble up and retry at the next boundary. This time it's successful. + 'Loading (outer)...', + 'Initial text', + ]); + expect(ReactNoop.getChildren()).toEqual([ + span('Loading (outer)...'), + div(span('Initial text')), + ]); + + // Unblock the inner boundary. + await advanceTimers(100); + expect(ReactNoop.flush()).toEqual([ + 'Promise resolved [Loading (inner)...]', + // Now the inner loading view should display, not the outer one. + 'Loading (inner)...', + 'Initial text', + ]); + expect(ReactNoop.getChildren()).toEqual([ + div(span('Loading (inner)...'), span('Initial text')), + ]); + + // Flush all the promises. + await advanceTimers(100); + + // Now the final result should display, with no loading state. + expect(ReactNoop.flush()).toEqual([ + 'Promise resolved [Final result]', + 'Final result', + ]); + expect(ReactNoop.getChildren()).toEqual([div(span('Final result'))]); + }); + + it('does not bubble through a boundary unless that boundary already captured', () => { + function App(props) { + return ( + + {isLoading => ( + + + {spinnerIsLoading => ( + + {spinnerIsLoading && } + {isLoading && } + + )} + + + {props.step > 0 && } + + )} + + ); + } + + ReactNoop.render(); + expect(ReactNoop.flush()).toEqual(['Initial text']); + expect(ReactNoop.getChildren()).toEqual([span('Initial text')]); + + ReactNoop.render(); + expect(ReactNoop.flush()).toEqual([ + 'Initial text', + 'Suspend! [More]', + 'Suspend! [(spinner)]', + 'Initial text', + '(fallback spinner)', + ]); + }); + + it('can resume a lower priority update', () => { + function App(props) { + return ( + + {isLoading => ( + + {isLoading ? : null} + {props.showContent ? ( + + ) : ( + + )} + + )} + + ); + } + + // Mount the initial view + ReactNoop.render(); + expect(ReactNoop.flush()).toEqual(['(empty)']); + expect(ReactNoop.getChildren()).toEqual([span('(empty)')]); + + // Toggle to show the content, which is async + ReactNoop.render(); + expect(ReactNoop.flush()).toEqual([ + // The content suspends because it's async + 'Suspend! [Content]', + // Show the loading view + 'Loading...', + '(empty)', + ]); + }); + it('can update at a higher priority while in a suspended state', async () => { function App(props) { return ( diff --git a/packages/react/src/React.js b/packages/react/src/React.js index d9805b24bd420..6576457331e9c 100644 --- a/packages/react/src/React.js +++ b/packages/react/src/React.js @@ -11,6 +11,7 @@ import { REACT_FRAGMENT_TYPE, REACT_STRICT_MODE_TYPE, REACT_ASYNC_MODE_TYPE, + REACT_LOADING_TYPE, REACT_TIMEOUT_TYPE, } from 'shared/ReactSymbols'; @@ -50,6 +51,7 @@ const React = { Fragment: REACT_FRAGMENT_TYPE, StrictMode: REACT_STRICT_MODE_TYPE, unstable_AsyncMode: REACT_ASYNC_MODE_TYPE, + Loading: REACT_LOADING_TYPE, Timeout: REACT_TIMEOUT_TYPE, createElement: __DEV__ ? createElementWithValidation : createElement, diff --git a/packages/react/src/ReactElementValidator.js b/packages/react/src/ReactElementValidator.js index 004e4b6deaf36..00ae9b78d5cd4 100644 --- a/packages/react/src/ReactElementValidator.js +++ b/packages/react/src/ReactElementValidator.js @@ -22,6 +22,7 @@ import { REACT_ASYNC_MODE_TYPE, REACT_PROVIDER_TYPE, REACT_CONTEXT_TYPE, + REACT_LOADING_TYPE, REACT_TIMEOUT_TYPE, } from 'shared/ReactSymbols'; import checkPropTypes from 'prop-types/checkPropTypes'; @@ -295,6 +296,7 @@ export function createElementWithValidation(type, props, children) { type === REACT_FRAGMENT_TYPE || type === REACT_ASYNC_MODE_TYPE || type === REACT_STRICT_MODE_TYPE || + type === REACT_LOADING_TYPE || type === REACT_TIMEOUT_TYPE || (typeof type === 'object' && type !== null && diff --git a/packages/shared/ReactSymbols.js b/packages/shared/ReactSymbols.js index f01c91899f9a3..4d39f3dd88975 100644 --- a/packages/shared/ReactSymbols.js +++ b/packages/shared/ReactSymbols.js @@ -36,9 +36,12 @@ export const REACT_CONTEXT_TYPE = hasSymbol export const REACT_ASYNC_MODE_TYPE = hasSymbol ? Symbol.for('react.async_mode') : 0xeacf; +export const REACT_LOADING_TYPE = hasSymbol + ? Symbol.for('react.loading') + : 0xeada; export const REACT_TIMEOUT_TYPE = hasSymbol ? Symbol.for('react.timeout') - : 0xeada; + : 0xeadb; const MAYBE_ITERATOR_SYMBOL = typeof Symbol === 'function' && Symbol.iterator; const FAUX_ITERATOR_SYMBOL = '@@iterator'; diff --git a/packages/shared/ReactTypeOfWork.js b/packages/shared/ReactTypeOfWork.js index 6abcca4217507..bb9dcd4066392 100644 --- a/packages/shared/ReactTypeOfWork.js +++ b/packages/shared/ReactTypeOfWork.js @@ -37,4 +37,5 @@ export const Fragment = 10; export const Mode = 11; export const ContextConsumer = 12; export const ContextProvider = 13; +export const LoadingComponent = 14; export const TimeoutComponent = 15;