diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js index 6e1272e42d66..985493ea4713 100644 --- a/packages/react-noop-renderer/src/createReactNoop.js +++ b/packages/react-noop-renderer/src/createReactNoop.js @@ -747,36 +747,27 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { return NoopRenderer.findHostInstance(component); }, - flushDeferredPri(timeout: number = Infinity): Array { - // The legacy version of this function decremented the timeout before - // returning the new time. - // TODO: Convert tests to use flushUnitsOfWork or flushAndYield instead. - const n = timeout / 5 - 1; - - let values = []; + flush(): Array { + let values = yieldedValues || []; + yieldedValues = null; // eslint-disable-next-line no-for-of-loops/no-for-of-loops - for (const value of flushUnitsOfWork(n)) { + for (const value of flushUnitsOfWork(Infinity)) { values.push(...value); } return values; }, - flush(): Array { - return ReactNoop.flushUnitsOfWork(Infinity); - }, - - flushAndYield( - unitsOfWork: number = Infinity, - ): Generator, void, void> { - return flushUnitsOfWork(unitsOfWork); - }, - - flushUnitsOfWork(n: number): Array { + // TODO: Should only be used via a Jest plugin (like we do with the + // test renderer). + unstable_flushNumberOfYields(n: number): Array { let values = yieldedValues || []; yieldedValues = null; // eslint-disable-next-line no-for-of-loops/no-for-of-loops - for (const value of flushUnitsOfWork(n)) { + for (const value of flushUnitsOfWork(Infinity)) { values.push(...value); + if (values.length >= n) { + break; + } } return values; }, @@ -847,7 +838,13 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { }, flushExpired(): Array { - return ReactNoop.flushUnitsOfWork(0); + let values = yieldedValues || []; + yieldedValues = null; + // eslint-disable-next-line no-for-of-loops/no-for-of-loops + for (const value of flushUnitsOfWork(0)) { + values.push(...value); + } + return values; }, yield(value: mixed) { diff --git a/packages/react-reconciler/src/ReactFiberScheduler.js b/packages/react-reconciler/src/ReactFiberScheduler.js index 4524d87b57b1..415d5bbc98ae 100644 --- a/packages/react-reconciler/src/ReactFiberScheduler.js +++ b/packages/react-reconciler/src/ReactFiberScheduler.js @@ -1214,7 +1214,7 @@ function workLoop(isYieldy) { } } else { // Flush asynchronous work until there's a higher priority event - while (nextUnitOfWork !== null && !shouldYieldToRenderer()) { + while (nextUnitOfWork !== null && !shouldYield()) { nextUnitOfWork = performUnitOfWork(nextUnitOfWork); } } @@ -2007,7 +2007,7 @@ function onSuspend( msUntilTimeout: number, ): void { root.expirationTime = rootExpirationTime; - if (msUntilTimeout === 0 && !shouldYieldToRenderer()) { + if (msUntilTimeout === 0 && !shouldYield()) { // Don't wait an additional tick. Commit the tree immediately. root.pendingCommitExpirationTime = suspendedExpirationTime; root.finishedWork = finishedWork; @@ -2204,43 +2204,24 @@ function findHighestPriorityRoot() { nextFlushedExpirationTime = highestPriorityWork; } -// TODO: This wrapper exists because many of the older tests (the ones that use -// flushDeferredPri) rely on the number of times `shouldYield` is called. We -// should get rid of it. -let didYield: boolean = false; -function shouldYieldToRenderer() { - if (didYield) { - return true; - } - if (shouldYield()) { - didYield = true; - return true; - } - return false; -} - function performAsyncWork(didTimeout) { - try { - if (didTimeout) { - // The callback timed out. That means at least one update has expired. - // Iterate through the root schedule. If they contain expired work, set - // the next render expiration time to the current time. This has the effect - // of flushing all expired work in a single batch, instead of flushing each - // level one at a time. - if (firstScheduledRoot !== null) { - recomputeCurrentRendererTime(); - let root: FiberRoot = firstScheduledRoot; - do { - didExpireAtExpirationTime(root, currentRendererTime); - // The root schedule is circular, so this is never null. - root = (root.nextScheduledRoot: any); - } while (root !== firstScheduledRoot); - } + if (didTimeout) { + // The callback timed out. That means at least one update has expired. + // Iterate through the root schedule. If they contain expired work, set + // the next render expiration time to the current time. This has the effect + // of flushing all expired work in a single batch, instead of flushing each + // level one at a time. + if (firstScheduledRoot !== null) { + recomputeCurrentRendererTime(); + let root: FiberRoot = firstScheduledRoot; + do { + didExpireAtExpirationTime(root, currentRendererTime); + // The root schedule is circular, so this is never null. + root = (root.nextScheduledRoot: any); + } while (root !== firstScheduledRoot); } - performWork(NoWork, true); - } finally { - didYield = false; } + performWork(NoWork, true); } function performSyncWork() { @@ -2266,7 +2247,7 @@ function performWork(minExpirationTime: ExpirationTime, isYieldy: boolean) { nextFlushedRoot !== null && nextFlushedExpirationTime !== NoWork && minExpirationTime <= nextFlushedExpirationTime && - !(didYield && currentRendererTime > nextFlushedExpirationTime) + !(shouldYield() && currentRendererTime > nextFlushedExpirationTime) ) { performWorkOnRoot( nextFlushedRoot, @@ -2414,7 +2395,7 @@ function performWorkOnRoot( if (finishedWork !== null) { // We've completed the root. Check the if we should yield one more time // before committing. - if (!shouldYieldToRenderer()) { + if (!shouldYield()) { // Still time left. Commit the root. completeRoot(root, finishedWork, expirationTime); } else { diff --git a/packages/react-reconciler/src/__tests__/ReactIncremental-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncremental-test.internal.js index 69bd1dd0aa90..1f65fc6ec3d9 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncremental-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactIncremental-test.internal.js @@ -39,10 +39,8 @@ describe('ReactIncremental', () => { }); it('should render a simple component, in steps if needed', () => { - let renderCallbackCalled = false; - let barCalled = false; function Bar() { - barCalled = true; + ReactNoop.yield('Bar'); return (
Hello World
@@ -50,26 +48,17 @@ describe('ReactIncremental', () => { ); } - let fooCalled = false; function Foo() { - fooCalled = true; + ReactNoop.yield('Foo'); return [, ]; } - ReactNoop.render(, () => (renderCallbackCalled = true)); - expect(fooCalled).toBe(false); - expect(barCalled).toBe(false); - expect(renderCallbackCalled).toBe(false); + ReactNoop.render(, () => ReactNoop.yield('callback')); // Do one step of work. - ReactNoop.flushDeferredPri(7 + 5); - expect(fooCalled).toBe(true); - expect(barCalled).toBe(false); - expect(renderCallbackCalled).toBe(false); + expect(ReactNoop.flushNextYield()).toEqual(['Foo']); + // Do the rest of the work. - ReactNoop.flushDeferredPri(50); - expect(fooCalled).toBe(true); - expect(barCalled).toBe(true); - expect(renderCallbackCalled).toBe(true); + expect(ReactNoop.flush()).toEqual(['Bar', 'Bar', 'callback']); }); it('updates a previous render', () => { @@ -140,15 +129,13 @@ describe('ReactIncremental', () => { }); it('can cancel partially rendered work and restart', () => { - let ops = []; - function Bar(props) { - ops.push('Bar'); + ReactNoop.yield('Bar'); return
{props.children}
; } function Foo(props) { - ops.push('Foo'); + ReactNoop.yield('Foo'); return (
{props.text} @@ -161,34 +148,22 @@ describe('ReactIncremental', () => { ReactNoop.render(); ReactNoop.flush(); - ops = []; - ReactNoop.render(); // Flush part of the work - ReactNoop.flushDeferredPri(20 + 5); - - expect(ops).toEqual(['Foo', 'Bar']); - - ops = []; + ReactNoop.flushThrough(['Foo', 'Bar']); // This will abort the previous work and restart ReactNoop.flushSync(() => ReactNoop.render(null)); ReactNoop.render(); - ReactNoop.clearYields(); // Flush part of the new work - ReactNoop.flushDeferredPri(20 + 5); - - expect(ops).toEqual(['Foo', 'Bar']); + ReactNoop.flushThrough(['Foo', 'Bar']); // Flush the rest of the work which now includes the low priority - ReactNoop.flush(20); - - expect(ops).toEqual(['Foo', 'Bar', 'Bar']); + expect(ReactNoop.flush()).toEqual(['Bar']); }); it('should call callbacks even if updates are aborted', () => { - const ops = []; let inst; class Foo extends React.Component { @@ -215,32 +190,27 @@ describe('ReactIncremental', () => { inst.setState( () => { - ops.push('setState1'); + ReactNoop.yield('setState1'); return {text: 'bar'}; }, - () => ops.push('callback1'), + () => ReactNoop.yield('callback1'), ); // Flush part of the work - ReactNoop.flushDeferredPri(20 + 5); - - expect(ops).toEqual(['setState1']); + ReactNoop.flushThrough(['setState1']); // This will abort the previous work and restart ReactNoop.flushSync(() => ReactNoop.render()); inst.setState( () => { - ops.push('setState2'); + ReactNoop.yield('setState2'); return {text2: 'baz'}; }, - () => ops.push('callback2'), + () => ReactNoop.yield('callback2'), ); // Flush the rest of the work which now includes the low priority - ReactNoop.flush(); - - expect(ops).toEqual([ - 'setState1', + expect(ReactNoop.flush()).toEqual([ 'setState1', 'setState2', 'callback1', @@ -250,20 +220,18 @@ describe('ReactIncremental', () => { }); it('can deprioritize unfinished work and resume it later', () => { - let ops = []; - function Bar(props) { - ops.push('Bar'); + ReactNoop.yield('Bar'); return
{props.children}
; } function Middle(props) { - ops.push('Middle'); + ReactNoop.yield('Middle'); return {props.children}; } function Foo(props) { - ops.push('Foo'); + ReactNoop.yield('Foo'); return (
{props.text} @@ -280,25 +248,20 @@ describe('ReactIncremental', () => { // Init ReactNoop.render(); - ReactNoop.flush(); - - expect(ops).toEqual(['Foo', 'Bar', 'Bar', 'Middle', 'Middle']); - - ops = []; + expect(ReactNoop.flush()).toEqual([ + 'Foo', + 'Bar', + 'Bar', + 'Middle', + 'Middle', + ]); // Render part of the work. This should be enough to flush everything except // the middle which has lower priority. ReactNoop.render(); - ReactNoop.flushDeferredPri(40); - - expect(ops).toEqual(['Foo', 'Bar', 'Bar']); - - ops = []; - + ReactNoop.flushThrough(['Foo', 'Bar', 'Bar']); // Flush only the remaining work - ReactNoop.flush(); - - expect(ops).toEqual(['Middle', 'Middle']); + expect(ReactNoop.flush()).toEqual(['Middle', 'Middle']); }); it('can deprioritize a tree from without dropping work', () => { @@ -1844,8 +1807,6 @@ describe('ReactIncremental', () => { }); it('merges and masks context', () => { - const ops = []; - class Intl extends React.Component { static childContextTypes = { locale: PropTypes.string, @@ -1856,7 +1817,7 @@ describe('ReactIncremental', () => { }; } render() { - ops.push('Intl ' + JSON.stringify(this.context)); + ReactNoop.yield('Intl ' + JSON.stringify(this.context)); return this.props.children; } } @@ -1871,7 +1832,7 @@ describe('ReactIncremental', () => { }; } render() { - ops.push('Router ' + JSON.stringify(this.context)); + ReactNoop.yield('Router ' + JSON.stringify(this.context)); return this.props.children; } } @@ -1881,7 +1842,7 @@ describe('ReactIncremental', () => { locale: PropTypes.string, }; render() { - ops.push('ShowLocale ' + JSON.stringify(this.context)); + ReactNoop.yield('ShowLocale ' + JSON.stringify(this.context)); return this.context.locale; } } @@ -1891,13 +1852,13 @@ describe('ReactIncremental', () => { route: PropTypes.string, }; render() { - ops.push('ShowRoute ' + JSON.stringify(this.context)); + ReactNoop.yield('ShowRoute ' + JSON.stringify(this.context)); return this.context.route; } } function ShowBoth(props, context) { - ops.push('ShowBoth ' + JSON.stringify(context)); + ReactNoop.yield('ShowBoth ' + JSON.stringify(context)); return `${context.route} in ${context.locale}`; } ShowBoth.contextTypes = { @@ -1907,14 +1868,14 @@ describe('ReactIncremental', () => { class ShowNeither extends React.Component { render() { - ops.push('ShowNeither ' + JSON.stringify(this.context)); + ReactNoop.yield('ShowNeither ' + JSON.stringify(this.context)); return null; } } class Indirection extends React.Component { render() { - ops.push('Indirection ' + JSON.stringify(this.context)); + ReactNoop.yield('Indirection ' + JSON.stringify(this.context)); return [ , , @@ -1927,7 +1888,6 @@ describe('ReactIncremental', () => { } } - ops.length = 0; ReactNoop.render( @@ -1936,18 +1896,18 @@ describe('ReactIncremental', () => {
, ); - expect(ReactNoop.flush).toWarnDev( + expect(() => + expect(ReactNoop.flush()).toEqual([ + 'Intl {}', + 'ShowLocale {"locale":"fr"}', + 'ShowBoth {"locale":"fr"}', + ]), + ).toWarnDev( 'Legacy context API has been detected within a strict-mode tree: \n\n' + 'Please update the following components: Intl, ShowBoth, ShowLocale', {withoutStack: true}, ); - expect(ops).toEqual([ - 'Intl {}', - 'ShowLocale {"locale":"fr"}', - 'ShowBoth {"locale":"fr"}', - ]); - ops.length = 0; ReactNoop.render( @@ -1956,14 +1916,12 @@ describe('ReactIncremental', () => {
, ); - ReactNoop.flush(); - expect(ops).toEqual([ + expect(ReactNoop.flush()).toEqual([ 'Intl {}', 'ShowLocale {"locale":"de"}', 'ShowBoth {"locale":"de"}', ]); - ops.length = 0; ReactNoop.render( @@ -1972,10 +1930,8 @@ describe('ReactIncremental', () => { , ); - ReactNoop.flushDeferredPri(15); - expect(ops).toEqual(['Intl {}']); + ReactNoop.flushThrough(['Intl {}']); - ops.length = 0; ReactNoop.render( @@ -1985,26 +1941,27 @@ describe('ReactIncremental', () => { , ); - expect(ReactNoop.flush).toWarnDev( + expect(() => + expect(ReactNoop.flush()).toEqual([ + 'ShowLocale {"locale":"sv"}', + 'ShowBoth {"locale":"sv"}', + 'Intl {}', + 'ShowLocale {"locale":"en"}', + 'Router {}', + 'Indirection {}', + 'ShowLocale {"locale":"en"}', + 'ShowRoute {"route":"/about"}', + 'ShowNeither {}', + 'Intl {}', + 'ShowBoth {"locale":"ru","route":"/about"}', + 'ShowBoth {"locale":"en","route":"/about"}', + 'ShowBoth {"locale":"en"}', + ]), + ).toWarnDev( 'Legacy context API has been detected within a strict-mode tree: \n\n' + 'Please update the following components: Router, ShowRoute', {withoutStack: true}, ); - expect(ops).toEqual([ - 'ShowLocale {"locale":"sv"}', - 'ShowBoth {"locale":"sv"}', - 'Intl {}', - 'ShowLocale {"locale":"en"}', - 'Router {}', - 'Indirection {}', - 'ShowLocale {"locale":"en"}', - 'ShowRoute {"route":"/about"}', - 'ShowNeither {}', - 'Intl {}', - 'ShowBoth {"locale":"ru","route":"/about"}', - 'ShowBoth {"locale":"en","route":"/about"}', - 'ShowBoth {"locale":"en"}', - ]); }); it('does not leak own context into context provider', () => { @@ -2080,8 +2037,6 @@ describe('ReactIncremental', () => { }); it('provides context when reusing work', () => { - const ops = []; - class Intl extends React.Component { static childContextTypes = { locale: PropTypes.string, @@ -2092,7 +2047,7 @@ describe('ReactIncremental', () => { }; } render() { - ops.push('Intl ' + JSON.stringify(this.context)); + ReactNoop.yield('Intl ' + JSON.stringify(this.context)); return this.props.children; } } @@ -2102,12 +2057,11 @@ describe('ReactIncremental', () => { locale: PropTypes.string, }; render() { - ops.push('ShowLocale ' + JSON.stringify(this.context)); + ReactNoop.yield('ShowLocale ' + JSON.stringify(this.context)); return this.context.locale; } } - ops.length = 0; ReactNoop.render( @@ -2120,24 +2074,23 @@ describe('ReactIncremental', () => { , ); - ReactNoop.flushDeferredPri(40); - expect(ops).toEqual([ + ReactNoop.flushThrough([ 'Intl {}', 'ShowLocale {"locale":"fr"}', 'ShowLocale {"locale":"fr"}', ]); - ops.length = 0; - expect(ReactNoop.flush).toWarnDev( + expect(() => + expect(ReactNoop.flush()).toEqual([ + 'ShowLocale {"locale":"fr"}', + 'Intl {}', + 'ShowLocale {"locale":"ru"}', + ]), + ).toWarnDev( 'Legacy context API has been detected within a strict-mode tree: \n\n' + 'Please update the following components: Intl, ShowLocale', {withoutStack: true}, ); - expect(ops).toEqual([ - 'ShowLocale {"locale":"fr"}', - 'Intl {}', - 'ShowLocale {"locale":"ru"}', - ]); }); it('reads context when setState is below the provider', () => { diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js index 43d2570c99f8..f4a24cfc0ae4 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js @@ -318,7 +318,7 @@ describe('ReactIncrementalErrorHandling', () => { ); } - ReactNoop.render(); + ReactNoop.render(, () => ReactNoop.yield('commit')); // Render the bad component asynchronously ReactNoop.flushThrough(['Parent', 'BadRender']); @@ -326,10 +326,9 @@ describe('ReactIncrementalErrorHandling', () => { // Finish the rest of the async work ReactNoop.flushThrough(['Sibling']); - // Rendering two more units of work should be enough to trigger the retry - // and synchronously throw an error. + // React retries once, synchronously, before throwing. ops = []; - expect(() => ReactNoop.flushUnitsOfWork(2)).toThrow('oops'); + expect(() => ReactNoop.flushNextYield()).toThrow('oops'); expect(ops).toEqual(['Parent', 'BadRender', 'Sibling']); }); @@ -409,7 +408,7 @@ describe('ReactIncrementalErrorHandling', () => { , ); - ReactNoop.flushDeferredPri(); + ReactNoop.flush(); expect(ReactNoop.getChildren()).toEqual([span('Caught an error: Hello.')]); }); @@ -588,20 +587,19 @@ describe('ReactIncrementalErrorHandling', () => { }); it('propagates an error from a noop error boundary during partial deferred mounting', () => { - const ops = []; class RethrowErrorBoundary extends React.Component { componentDidCatch(error) { - ops.push('RethrowErrorBoundary componentDidCatch'); + ReactNoop.yield('RethrowErrorBoundary componentDidCatch'); throw error; } render() { - ops.push('RethrowErrorBoundary render'); + ReactNoop.yield('RethrowErrorBoundary render'); return this.props.children; } } function BrokenRender() { - ops.push('BrokenRender'); + ReactNoop.yield('BrokenRender'); throw new Error('Hello'); } @@ -611,16 +609,12 @@ describe('ReactIncrementalErrorHandling', () => { , ); - ReactNoop.flushDeferredPri(15); - expect(ops).toEqual(['RethrowErrorBoundary render']); + ReactNoop.flushThrough(['RethrowErrorBoundary render']); - ops.length = 0; expect(() => { ReactNoop.flush(); }).toThrow('Hello'); - expect(ops).toEqual([ - 'BrokenRender', - + expect(ReactNoop.clearYields()).toEqual([ // React retries one more time 'RethrowErrorBoundary render', 'BrokenRender', @@ -1526,7 +1520,7 @@ describe('ReactIncrementalErrorHandling', () => { , ); - ReactNoop.flushDeferredPri(); + ReactNoop.flush(); expect(ReactNoop.getChildren()).toEqual([ span( 'Caught an error:\n' + @@ -1562,7 +1556,7 @@ describe('ReactIncrementalErrorHandling', () => { , ); - ReactNoop.flushDeferredPri(); + ReactNoop.flush(); expect(ReactNoop.getChildren()).toEqual([span('Caught an error: Hello')]); }); diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorLogging-test.js b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorLogging-test.js index 06f38de82910..4394b2822e79 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorLogging-test.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorLogging-test.js @@ -50,7 +50,7 @@ describe('ReactIncrementalErrorLogging', () => {
, ); - expect(ReactNoop.flushDeferredPri).toThrowError('constructor error'); + expect(() => ReactNoop.flush()).toThrowError('constructor error'); expect(console.error).toHaveBeenCalledTimes(1); expect(console.error).toHaveBeenCalledWith( __DEV__ @@ -86,7 +86,7 @@ describe('ReactIncrementalErrorLogging', () => { , ); - expect(ReactNoop.flushDeferredPri).toThrowError('componentDidMount error'); + expect(() => ReactNoop.flush()).toThrowError('componentDidMount error'); expect(console.error).toHaveBeenCalledTimes(1); expect(console.error).toHaveBeenCalledWith( __DEV__ @@ -125,7 +125,7 @@ describe('ReactIncrementalErrorLogging', () => { , ); - expect(ReactNoop.flushDeferredPri).toThrow('render error'); + expect(() => ReactNoop.flush()).toThrow('render error'); expect(logCapturedErrorCalls.length).toBe(1); expect(logCapturedErrorCalls[0]).toEqual( __DEV__ diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalPerf-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncrementalPerf-test.internal.js index 01a462debb6f..c3b18696380b 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalPerf-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalPerf-test.internal.js @@ -117,6 +117,7 @@ describe('ReactDebugFiberPerf', () => { require('shared/ReactFeatureFlags').enableUserTimingAPI = true; require('shared/ReactFeatureFlags').enableProfilerTimer = false; require('shared/ReactFeatureFlags').replayFailedUnitOfWorkWithInvokeGuardedCallback = false; + require('shared/ReactFeatureFlags').debugRenderPhaseSideEffectsForStrictMode = false; // Import after the polyfill is set up: React = require('react'); @@ -397,12 +398,21 @@ describe('ReactDebugFiberPerf', () => { it('measures deferred work in chunks', () => { class A extends React.Component { render() { + ReactNoop.yield('A'); return
{this.props.children}
; } } class B extends React.Component { render() { + ReactNoop.yield('B'); + return
{this.props.children}
; + } + } + + class C extends React.Component { + render() { + ReactNoop.yield('C'); return
{this.props.children}
; } } @@ -415,14 +425,15 @@ describe('ReactDebugFiberPerf', () => { + + + , ); - addComment('Start mounting Parent and A'); - ReactNoop.flushDeferredPri(40); - addComment('Mount B just a little (but not enough to memoize)'); - ReactNoop.flushDeferredPri(10); - addComment('Complete B and Parent'); - ReactNoop.flushDeferredPri(); + addComment('Start rendering through B'); + ReactNoop.flushThrough(['A', 'B']); + addComment('Complete the rest'); + ReactNoop.flush(); expect(getFlameChart()).toMatchSnapshot(); }); @@ -632,11 +643,12 @@ describe('ReactDebugFiberPerf', () => { it('warns if an in-progress update is interrupted', () => { function Foo() { + ReactNoop.yield('Foo'); return ; } ReactNoop.render(); - ReactNoop.flushUnitsOfWork(2); + ReactNoop.flushNextYield(); ReactNoop.flushSync(() => { ReactNoop.render(); }); diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalReflection-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncrementalReflection-test.internal.js index fa3dc6e7e013..c82b2f990ef2 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalReflection-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalReflection-test.internal.js @@ -24,8 +24,6 @@ describe('ReactIncrementalReflection', () => { }); it('handles isMounted even when the initial render is deferred', () => { - let ops = []; - const instances = []; class Component extends React.Component { @@ -36,10 +34,10 @@ describe('ReactIncrementalReflection', () => { } UNSAFE_componentWillMount() { instances.push(this); - ops.push('componentWillMount', this._isMounted()); + ReactNoop.yield('componentWillMount: ' + this._isMounted()); } componentDidMount() { - ops.push('componentDidMount', this._isMounted()); + ReactNoop.yield('componentDidMount: ' + this._isMounted()); } render() { return ; @@ -53,29 +51,23 @@ describe('ReactIncrementalReflection', () => { ReactNoop.render(); // Render part way through but don't yet commit the updates. - ReactNoop.flushDeferredPri(20); - - expect(ops).toEqual(['componentWillMount', false]); + ReactNoop.flushThrough(['componentWillMount: false']); expect(instances[0]._isMounted()).toBe(false); - ops = []; - // Render the rest and commit the updates. - expect(ReactNoop.flush).toWarnDev( + expect(() => + expect(ReactNoop.flush()).toEqual(['componentDidMount: true']), + ).toWarnDev( 'componentWillMount: Please update the following components ' + 'to use componentDidMount instead: Component', {withoutStack: true}, ); - expect(ops).toEqual(['componentDidMount', true]); - expect(instances[0]._isMounted()).toBe(true); }); it('handles isMounted when an unmount is deferred', () => { - let ops = []; - const instances = []; class Component extends React.Component { @@ -86,16 +78,16 @@ describe('ReactIncrementalReflection', () => { instances.push(this); } componentWillUnmount() { - ops.push('componentWillUnmount', this._isMounted()); + ReactNoop.yield('componentWillUnmount: ' + this._isMounted()); } render() { - ops.push('Component'); + ReactNoop.yield('Component'); return ; } } function Other() { - ops.push('Other'); + ReactNoop.yield('Other'); return ; } @@ -104,38 +96,28 @@ describe('ReactIncrementalReflection', () => { } ReactNoop.render(); - expect(ReactNoop.flush).toWarnDev( + expect(() => expect(ReactNoop.flush()).toEqual(['Component'])).toWarnDev( 'componentWillMount: Please update the following components ' + 'to use componentDidMount instead: Component', {withoutStack: true}, ); - expect(ops).toEqual(['Component']); - ops = []; - expect(instances[0]._isMounted()).toBe(true); ReactNoop.render(); // Render part way through but don't yet commit the updates so it is not // fully unmounted yet. - ReactNoop.flushDeferredPri(20); - - expect(ops).toEqual(['Other']); - ops = []; + ReactNoop.flushThrough(['Other']); expect(instances[0]._isMounted()).toBe(true); // Finish flushing the unmount. - ReactNoop.flush(); - - expect(ops).toEqual(['componentWillUnmount', true]); + expect(ReactNoop.flush()).toEqual(['componentWillUnmount: true']); expect(instances[0]._isMounted()).toBe(false); }); it('finds no node before insertion and correct node before deletion', () => { - let ops = []; - let classInstance = null; function findInstance(inst) { @@ -154,22 +136,22 @@ describe('ReactIncrementalReflection', () => { class Component extends React.Component { UNSAFE_componentWillMount() { classInstance = this; - ops.push('componentWillMount', findInstance(this)); + ReactNoop.yield(['componentWillMount', findInstance(this)]); } componentDidMount() { - ops.push('componentDidMount', findInstance(this)); + ReactNoop.yield(['componentDidMount', findInstance(this)]); } UNSAFE_componentWillUpdate() { - ops.push('componentWillUpdate', findInstance(this)); + ReactNoop.yield(['componentWillUpdate', findInstance(this)]); } componentDidUpdate() { - ops.push('componentDidUpdate', findInstance(this)); + ReactNoop.yield(['componentDidUpdate', findInstance(this)]); } componentWillUnmount() { - ops.push('componentWillUnmount', findInstance(this)); + ReactNoop.yield(['componentWillUnmount', findInstance(this)]); } render() { - ops.push('render'); + ReactNoop.yield('render'); return this.props.step < 2 ? ( (this.span = ref)} /> ) : this.props.step === 2 ? ( @@ -182,7 +164,7 @@ describe('ReactIncrementalReflection', () => { function Sibling() { // Sibling is used to assert that we've rendered past the first component. - ops.push('render sibling'); + ReactNoop.yield('render sibling'); return ; } @@ -192,23 +174,22 @@ describe('ReactIncrementalReflection', () => { ReactNoop.render(); // Flush past Component but don't complete rendering everything yet. - ReactNoop.flushDeferredPri(30); - - expect(ops).toEqual([ - 'componentWillMount', - null, + ReactNoop.flushThrough([ + ['componentWillMount', null], 'render', 'render sibling', ]); - ops = []; - expect(classInstance).toBeDefined(); // The instance has been complete but is still not committed so it should // not find any host nodes in it. expect(findInstance(classInstance)).toBe(null); - expect(ReactNoop.flush).toWarnDev( + expect(() => + expect(ReactNoop.flush()).toEqual([ + ['componentDidMount', classInstance.span], + ]), + ).toWarnDev( 'componentWillMount: Please update the following components ' + 'to use componentDidMount instead: Component' + '\n\ncomponentWillUpdate: Please update the following components ' + @@ -221,79 +202,54 @@ describe('ReactIncrementalReflection', () => { expect(findInstance(classInstance)).toBe(hostSpan); - expect(ops).toEqual(['componentDidMount', hostSpan]); - - ops = []; - // Flush next step which will cause an update but not yet render a new host // node. ReactNoop.render(); - ReactNoop.flush(); - - expect(ops).toEqual([ - 'componentWillUpdate', - hostSpan, + expect(ReactNoop.flush()).toEqual([ + ['componentWillUpdate', hostSpan], 'render', 'render sibling', - 'componentDidUpdate', - hostSpan, + ['componentDidUpdate', hostSpan], ]); expect(ReactNoop.findInstance(classInstance)).toBe(hostSpan); - ops = []; - // The next step will render a new host node but won't get committed yet. // We expect this to mutate the original Fiber. ReactNoop.render(); - ReactNoop.flushDeferredPri(30); - - expect(ops).toEqual([ - 'componentWillUpdate', - hostSpan, + ReactNoop.flushThrough([ + ['componentWillUpdate', hostSpan], 'render', 'render sibling', ]); - ops = []; - // This should still be the host span. expect(ReactNoop.findInstance(classInstance)).toBe(hostSpan); // When we finally flush the tree it will get committed. - ReactNoop.flush(); + expect(ReactNoop.flush()).toEqual([ + ['componentDidUpdate', classInstance.div], + ]); const hostDiv = classInstance.div; - expect(hostDiv).toBeDefined(); expect(hostSpan).not.toBe(hostDiv); - expect(ops).toEqual(['componentDidUpdate', hostDiv]); - - ops = []; - // We should now find the new host node. expect(ReactNoop.findInstance(classInstance)).toBe(hostDiv); // Render to null but don't commit it yet. ReactNoop.render(); - ReactNoop.flushDeferredPri(25); - - expect(ops).toEqual([ - 'componentWillUpdate', - hostDiv, + ReactNoop.flushThrough([ + ['componentWillUpdate', hostDiv], 'render', 'render sibling', ]); - ops = []; - // This should still be the host div since the deletion is not committed. expect(ReactNoop.findInstance(classInstance)).toBe(hostDiv); - ReactNoop.flush(); - - expect(ops).toEqual(['componentDidUpdate', null]); + expect(ReactNoop.flush()).toEqual([['componentDidUpdate', null]]); // This should still be the host div since the deletion is not committed. expect(ReactNoop.findInstance(classInstance)).toBe(null); @@ -302,11 +258,8 @@ describe('ReactIncrementalReflection', () => { ReactNoop.render(); ReactNoop.flush(); - ops = []; - // Unmount the component. ReactNoop.render([]); - ReactNoop.flush(); - expect(ops).toEqual(['componentWillUnmount', hostDiv]); + expect(ReactNoop.flush()).toEqual([['componentWillUnmount', hostDiv]]); }); }); diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalScheduling-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncrementalScheduling-test.internal.js index b362585a5a5c..353a20ce65b2 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalScheduling-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalScheduling-test.internal.js @@ -31,7 +31,7 @@ describe('ReactIncrementalScheduling', () => { ReactNoop.render(); expect(ReactNoop.getChildren()).toEqual([]); - ReactNoop.flushDeferredPri(); + ReactNoop.flush(); expect(ReactNoop.getChildren()).toEqual([span('1')]); }); @@ -86,71 +86,99 @@ describe('ReactIncrementalScheduling', () => { }); it('works on deferred roots in the order they were scheduled', () => { - ReactNoop.renderToRootWithID(, 'a'); - ReactNoop.renderToRootWithID(, 'b'); - ReactNoop.renderToRootWithID(, 'c'); + const {useEffect} = React; + function Text({text}) { + useEffect( + () => { + ReactNoop.yield(text); + }, + [text], + ); + return text; + } + + ReactNoop.act(() => { + ReactNoop.renderToRootWithID(, 'a'); + ReactNoop.renderToRootWithID(, 'b'); + ReactNoop.renderToRootWithID(, 'c'); + }); ReactNoop.flush(); - expect(ReactNoop.getChildren('a')).toEqual([span('a:1')]); - expect(ReactNoop.getChildren('b')).toEqual([span('b:1')]); - expect(ReactNoop.getChildren('c')).toEqual([span('c:1')]); + + expect(ReactNoop.getChildrenAsJSX('a')).toEqual('a:1'); + expect(ReactNoop.getChildrenAsJSX('b')).toEqual('b:1'); + expect(ReactNoop.getChildrenAsJSX('c')).toEqual('c:1'); // Schedule deferred work in the reverse order - ReactNoop.renderToRootWithID(, 'c'); - ReactNoop.renderToRootWithID(, 'b'); + ReactNoop.act(() => { + ReactNoop.renderToRootWithID(, 'c'); + ReactNoop.renderToRootWithID(, 'b'); + }); // Ensure it starts in the order it was scheduled - ReactNoop.flushDeferredPri(15 + 5); - expect(ReactNoop.getChildren('a')).toEqual([span('a:1')]); - expect(ReactNoop.getChildren('b')).toEqual([span('b:1')]); - expect(ReactNoop.getChildren('c')).toEqual([span('c:2')]); + ReactNoop.flushThrough(['c:2']); + + expect(ReactNoop.getChildrenAsJSX('a')).toEqual('a:1'); + expect(ReactNoop.getChildrenAsJSX('b')).toEqual('b:1'); + expect(ReactNoop.getChildrenAsJSX('c')).toEqual('c:2'); // Schedule last bit of work, it will get processed the last - ReactNoop.renderToRootWithID(, 'a'); + ReactNoop.act(() => { + ReactNoop.renderToRootWithID(, 'a'); + }); // Keep performing work in the order it was scheduled - ReactNoop.flushDeferredPri(15 + 5); - expect(ReactNoop.getChildren('a')).toEqual([span('a:1')]); - expect(ReactNoop.getChildren('b')).toEqual([span('b:2')]); - expect(ReactNoop.getChildren('c')).toEqual([span('c:2')]); - ReactNoop.flushDeferredPri(15 + 5); - expect(ReactNoop.getChildren('a')).toEqual([span('a:2')]); - expect(ReactNoop.getChildren('b')).toEqual([span('b:2')]); - expect(ReactNoop.getChildren('c')).toEqual([span('c:2')]); + ReactNoop.flushThrough(['b:2']); + expect(ReactNoop.getChildrenAsJSX('a')).toEqual('a:1'); + expect(ReactNoop.getChildrenAsJSX('b')).toEqual('b:2'); + expect(ReactNoop.getChildrenAsJSX('c')).toEqual('c:2'); + + ReactNoop.flushThrough(['a:2']); + expect(ReactNoop.getChildrenAsJSX('a')).toEqual('a:2'); + expect(ReactNoop.getChildrenAsJSX('b')).toEqual('b:2'); + expect(ReactNoop.getChildrenAsJSX('c')).toEqual('c:2'); }); it('schedules sync updates when inside componentDidMount/Update', () => { let instance; - let ops = []; class Foo extends React.Component { state = {tick: 0}; componentDidMount() { - ops.push('componentDidMount (before setState): ' + this.state.tick); + ReactNoop.yield( + 'componentDidMount (before setState): ' + this.state.tick, + ); this.setState({tick: 1}); // We're in a batch. Update hasn't flushed yet. - ops.push('componentDidMount (after setState): ' + this.state.tick); + ReactNoop.yield( + 'componentDidMount (after setState): ' + this.state.tick, + ); } componentDidUpdate() { - ops.push('componentDidUpdate: ' + this.state.tick); + ReactNoop.yield('componentDidUpdate: ' + this.state.tick); if (this.state.tick === 2) { - ops.push('componentDidUpdate (before setState): ' + this.state.tick); + ReactNoop.yield( + 'componentDidUpdate (before setState): ' + this.state.tick, + ); this.setState({tick: 3}); - ops.push('componentDidUpdate (after setState): ' + this.state.tick); + ReactNoop.yield( + 'componentDidUpdate (after setState): ' + this.state.tick, + ); // We're in a batch. Update hasn't flushed yet. } } render() { - ops.push('render: ' + this.state.tick); + ReactNoop.yield('render: ' + this.state.tick); instance = this; return ; } } ReactNoop.render(); + // Render without committing + ReactNoop.flushThrough(['render: 0']); - ReactNoop.flushDeferredPri(20 + 5); - expect(ops).toEqual([ - 'render: 0', + // Do one more unit of work to commit + expect(ReactNoop.flushNextYield()).toEqual([ 'componentDidMount (before setState): 0', 'componentDidMount (after setState): 0', // If the setState inside componentDidMount were deferred, there would be @@ -159,12 +187,9 @@ describe('ReactIncrementalScheduling', () => { 'componentDidUpdate: 1', ]); - ops = []; instance.setState({tick: 2}); - ReactNoop.flushDeferredPri(20 + 5); - - expect(ops).toEqual([ - 'render: 2', + ReactNoop.flushThrough(['render: 2']); + expect(ReactNoop.flushNextYield()).toEqual([ 'componentDidUpdate: 2', 'componentDidUpdate (before setState): 2', 'componentDidUpdate (after setState): 2', @@ -254,17 +279,18 @@ describe('ReactIncrementalScheduling', () => { }); } render() { + ReactNoop.yield('Foo'); return ; } } ReactNoop.render(); // This should be just enough to complete all the work, but not enough to // commit it. - ReactNoop.flushDeferredPri(20); + ReactNoop.flushThrough(['Foo']); expect(ReactNoop.getChildren()).toEqual([]); // Do one more unit of work. - ReactNoop.flushDeferredPri(10); + ReactNoop.flushNextYield(); // The updates should all be flushed with Task priority expect(ReactNoop.getChildren()).toEqual([span(5)]); }); diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalSideEffects-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncrementalSideEffects-test.internal.js index f5195298614e..9e14dd202fb5 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalSideEffects-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalSideEffects-test.internal.js @@ -373,10 +373,12 @@ describe('ReactIncrementalSideEffects', () => { it('does not update child nodes if a flush is aborted', () => { function Bar(props) { + ReactNoop.yield('Bar'); return ; } function Foo(props) { + ReactNoop.yield('Foo'); return (
@@ -395,7 +397,9 @@ describe('ReactIncrementalSideEffects', () => { ]); ReactNoop.render(); - ReactNoop.flushDeferredPri(35); + + // Flush some of the work without committing + ReactNoop.flushThrough(['Foo', 'Bar']); expect(ReactNoop.getChildren()).toEqual([ div(div(span('Hello'), span('Hello')), span('Yo')), ]); @@ -403,10 +407,12 @@ describe('ReactIncrementalSideEffects', () => { it('preserves a previously rendered node when deprioritized', () => { function Middle(props) { + ReactNoop.yield('Middle'); return ; } function Foo(props) { + ReactNoop.yield('Foo'); return (
, ); - ReactNoop.render(); - ReactNoop.flushDeferredPri(20); - + ReactNoop.render(, () => ReactNoop.yield('commit')); + ReactNoop.flushThrough(['Foo', 'commit']); expect(ReactNoop.getChildrenAsJSX()).toEqual(
, ); - ReactNoop.flush(); + ReactNoop.flush(); expect(ReactNoop.getChildrenAsJSX()).toEqual(