Replace noop's fake Scheduler implementation with mock Scheduler build (#14969)

* Replace noop's fake Scheduler implementation with mock Scheduler build
The noop renderer has its own mock implementation of the Scheduler
interface, with the ability to partially render work in tests. Now that
this functionality has been lifted into a proper mock Scheduler build,
we can use that instead.
Most of the existing noop tests were unaffected, but I did have to make
some changes. The biggest one involved passive effects: previously, they
were scheduled on a separate queue from the queue that handles
rendering. After this change, both rendering and effects are scheduled
in the Scheduler queue. I think this is a better approach because tests
no longer have to worry about the difference; if you call `flushAll`,
all the work is flushed, both rendering and effects. But for those few
tests that do care to flush the rendering without the effects, that's
still possible using the `yieldValue` API.
Follow-up: Do the same for test renderer.
* Fix import to scheduler/unstable_mock

Author: acdlite

packages/react-noop-renderer/src/createReactNoop.js

@@ -18,6 +18,7 @@ import type {Fiber} from 'react-reconciler/src/ReactFiber';
 importtype{UpdateQueue}from'react-reconciler/src/ReactUpdateQueue';
 importtype{ReactNodeList}from'shared/ReactTypes';
 
+import*asSchedulerfrom'scheduler/unstable_mock';
 import{createPortal}from'shared/ReactPortal';
 importexpectfrom'expect';
 import{REACT_FRAGMENT_TYPE,REACT_ELEMENT_TYPE}from'shared/ReactSymbols';
@@ -51,9 +52,6 @@ if (__DEV__) {
 }
 
 functioncreateReactNoop(reconciler: Function,useMutation: boolean){
-letscheduledCallback=null;
-letscheduledCallbackTimeout=-1;
-letscheduledPassiveCallback=null;
 letinstanceCounter=0;
 lethostDiffCounter=0;
 lethostUpdateCounter=0;
@@ -218,8 +216,6 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
 );
 }
 
-letelapsedTimeInMs=0;
-
 constsharedHostConfig={
 getRootHostContext(){
 returnNO_CONTEXT;
@@ -308,66 +304,23 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
 returninst;
 },
 
-scheduleDeferredCallback(callback,options){
-if(scheduledCallback){
-thrownewError(
-'Scheduling a callback twice is excessive. Instead, keep track of '+
-'whether the callback has already been scheduled.',
-);
-}
-scheduledCallback=callback;
-if(
-typeofoptions==='object'&&
-options!==null&&
-typeofoptions.timeout==='number'
-){
-const newTimeout =options.timeout;
-if(
-scheduledCallbackTimeout===-1||
-scheduledCallbackTimeout>newTimeout
-){
- scheduledCallbackTimeout =elapsedTimeInMs+newTimeout;
-}
-}
-return0;
-},
-
-cancelDeferredCallback(){
-if(scheduledCallback===null){
-thrownewError('No callback is scheduled.');
-}
- scheduledCallback =null;
-scheduledCallbackTimeout=-1;
-},
+scheduleDeferredCallback: Scheduler.unstable_scheduleCallback,
+cancelDeferredCallback: Scheduler.unstable_cancelCallback,
 
-shouldYield,
+shouldYield: Scheduler.unstable_shouldYield,
 
 scheduleTimeout: setTimeout,
 cancelTimeout: clearTimeout,
 noTimeout: -1,
-schedulePassiveEffects(callback){
-if(scheduledCallback){
-thrownewError(
-'Scheduling a callback twice is excessive. Instead, keep track of '+
-'whether the callback has already been scheduled.',
-);
-}
-scheduledPassiveCallback=callback;
-},
-cancelPassiveEffects(){
-if(scheduledPassiveCallback===null){
-thrownewError('No passive effects callback is scheduled.');
-}
- scheduledPassiveCallback =null;
-},
+
+schedulePassiveEffects: Scheduler.unstable_scheduleCallback,
+cancelPassiveEffects: Scheduler.unstable_cancelCallback,
 
 prepareForCommit(): void{},
 
 resetAfterCommit(): void{},
 
-now(): number{
-returnelapsedTimeInMs;
-},
+now: Scheduler.unstable_now,
 
 isPrimaryRenderer: true,
 supportsHydration: false,
@@ -534,71 +487,6 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
 constroots=newMap();
 constDEFAULT_ROOT_ID= '<default>';
 
-letyieldedValues: Array<mixed>=[];
- let didStop: boolean=false;
- let expectedNumberOfYields: number=-1;
-
-functionshouldYield(){
-if(
-expectedNumberOfYields!==-1&&
-yieldedValues.length>=expectedNumberOfYields&&
-(scheduledCallbackTimeout===-1||
-elapsedTimeInMs<scheduledCallbackTimeout)
-){
-// We yielded at least as many values as expected. Stop rendering.
-didStop=true;
-returntrue;
-}
-// Keep rendering.
-returnfalse;
-}
-
-functionflushAll(): Array<mixed>{
-yieldedValues=[];
-while(scheduledCallback!==null){
-constcb=scheduledCallback;
-scheduledCallback=null;
-constdidTimeout=
-scheduledCallbackTimeout!==-1&&
-scheduledCallbackTimeout<elapsedTimeInMs;
-cb(didTimeout);
-}
-constvalues=yieldedValues;
-yieldedValues=[];
-returnvalues;
-}
-
-functionflushNumberOfYields(count: number): Array<mixed>{
-expectedNumberOfYields=count;
-didStop=false;
-yieldedValues=[];
-try{
-while(scheduledCallback!==null&&!didStop){
-constcb=scheduledCallback;
-scheduledCallback=null;
-constdidTimeout=
-scheduledCallbackTimeout!==-1&&
-scheduledCallbackTimeout<elapsedTimeInMs;
-cb(didTimeout);
-}
-returnyieldedValues;
-}finally{
-expectedNumberOfYields=-1;
-didStop=false;
-yieldedValues=[];
-}
-}
-
-functionyieldValue(value: mixed): void{
-yieldedValues.push(value);
-}
-
-functionclearYields(): Array<mixed>{
-constvalues=yieldedValues;
-yieldedValues=[];
-returnvalues;
-}
-
 functionchildToJSX(child,text){
 if(text!==null){
 returntext;
@@ -653,6 +541,8 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
 }
 
 constReactNoop={
+_Scheduler: Scheduler,
+
 getChildren(rootID: string=DEFAULT_ROOT_ID){
 constcontainer=rootContainers.get(rootID);
 if(container){
@@ -763,14 +653,9 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
 returnNoopRenderer.findHostInstance(component);
 },
 
-// TODO: Should only be used via a Jest plugin (like we do with the
-// test renderer).
- unstable_flushWithoutYielding: flushAll,
-unstable_flushNumberOfYields: flushNumberOfYields,
-unstable_clearYields: clearYields,
-
 flushNextYield(): Array<mixed>{
-returnflushNumberOfYields(1);
+ Scheduler.unstable_flushNumberOfYields(1);
+returnScheduler.unstable_clearYields();
 },
 
 flushWithHostCounters(
@@ -788,7 +673,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
 hostUpdateCounter=0;
 hostCloneCounter=0;
 try{
-flushAll();
+Scheduler.flushAll();
 returnuseMutation
  ? {
 hostDiffCounter,
@@ -805,24 +690,13 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
 }
 },
 
-expire(ms: number): Array<mixed>{
- ReactNoop.advanceTime(ms);
-returnReactNoop.flushExpired();
-},
-
-advanceTime(ms: number): void{
- elapsedTimeInMs +=ms;
-},
+expire: Scheduler.advanceTime,
 
 flushExpired(): Array<mixed>{
-returnflushNumberOfYields(0);
+returnScheduler.unstable_flushExpired();
 },
 
-yield: yieldValue,
-
-hasScheduledCallback(){
-return!!scheduledCallback;
-},
+yield: Scheduler.yieldValue,
 
 batchedUpdates: NoopRenderer.batchedUpdates,
 
@@ -870,9 +744,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
 },
 
 flushSync(fn: ()=>mixed){
-yieldedValues=[];
 NoopRenderer.flushSync(fn);
-returnyieldedValues;
 },
 
 flushPassiveEffects(){
@@ -997,12 +869,13 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
 _next: null,
 };
 root.firstBatch=batch;
-constactual=flushAll();
+Scheduler.unstable_flushWithoutYielding();
+constactual=Scheduler.unstable_clearYields();
 expect(actual).toEqual(expectedFlush);
 return(expectedCommit: Array<mixed>)=>{
  batch._defer=false;
 NoopRenderer.flushRoot(root,expiration);
-expect(yieldedValues).toEqual(expectedCommit);
+expect(Scheduler.unstable_clearYields()).toEqual(expectedCommit);
 };
 },
 

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

@@ -12,6 +12,7 @@
 letReact;
 letReactFeatureFlags;
 letReactNoop;
+letScheduler;
 
 describe('ReactExpiration',()=>{
 beforeEach(()=>{
@@ -20,6 +21,7 @@ describe('ReactExpiration', () => {
 ReactFeatureFlags.debugRenderPhaseSideEffectsForStrictMode=false;
 React=require('react');
 ReactNoop=require('react-noop-renderer');
+Scheduler=require('scheduler');
 });
 
 functionspan(prop){
@@ -60,19 +62,33 @@ describe('ReactExpiration', () => {
 }
 }
 
+functioninterrupt(){
+ReactNoop.flushSync(()=>{
+ReactNoop.renderToRootWithID(null,'other-root');
+});
+}
+
 // First, show what happens for updates in two separate events.
 // Schedule an update.
 ReactNoop.render(<Texttext="A"/>);
-// Advance the timer and flush any work that expired. Flushing the expired
-// work signals to the renderer that the event has ended.
-ReactNoop.advanceTime(2000);
+// Advance the timer.
+Scheduler.advanceTime(2000);
+// Partially flush the the first update, then interrupt it.
+expect(ReactNoop).toFlushAndYieldThrough(['A [render]']);
+interrupt();
+
 // Don't advance time by enough to expire the first update.
-expect(ReactNoop.flushExpired()).toEqual([]);
+expect(Scheduler).toHaveYielded([]);
 expect(ReactNoop.getChildren()).toEqual([]);
+
 // Schedule another update.
 ReactNoop.render(<Texttext="B"/>);
 // The updates should flush in separate batches, since sufficient time
 // passed in between them *and* they occurred in separate events.
+// Note: This isn't necessarily the ideal behavior. It might be better to
+// batch these two updates together. The fact that they aren't batched
+// is an implementation detail. The important part of this unit test is that
+// they are batched if it's possible that they happened in the same event.
 expect(ReactNoop).toFlushAndYield([
 'A [render]',
 'A [commit]',
@@ -84,10 +100,7 @@ describe('ReactExpiration', () => {
 // Now do the same thing again, except this time don't flush any work in
 // between the two updates.
 ReactNoop.render(<Texttext="A"/>);
-// Advance the timer, but don't flush the expired work. Because we still
-// haven't entered an idle callback, the scheduler must assume that we're
-// inside the same event.
-ReactNoop.advanceTime(2000);
+Scheduler.advanceTime(2000);
 expect(ReactNoop).toHaveYielded([]);
 expect(ReactNoop.getChildren()).toEqual([span('B')]);
 // Schedule another update.
@@ -114,19 +127,33 @@ describe('ReactExpiration', () => {
 }
 }
 
+functioninterrupt(){
+ReactNoop.flushSync(()=>{
+ReactNoop.renderToRootWithID(null,'other-root');
+});
+}
+
 // First, show what happens for updates in two separate events.
 // Schedule an update.
 ReactNoop.render(<Texttext="A"/>);
-// Advance the timer and flush any work that expired. Flushing the expired
-// work signals to the renderer that the event has ended.
-ReactNoop.advanceTime(2000);
+// Advance the timer.
+Scheduler.advanceTime(2000);
+// Partially flush the the first update, then interrupt it.
+expect(ReactNoop).toFlushAndYieldThrough(['A [render]']);
+interrupt();
+
 // Don't advance time by enough to expire the first update.
-expect(ReactNoop.flushExpired()).toEqual([]);
+expect(Scheduler).toHaveYielded([]);
 expect(ReactNoop.getChildren()).toEqual([]);
+
 // Schedule another update.
 ReactNoop.render(<Texttext="B"/>);
 // The updates should flush in separate batches, since sufficient time
 // passed in between them *and* they occurred in separate events.
+// Note: This isn't necessarily the ideal behavior. It might be better to
+// batch these two updates together. The fact that they aren't batched
+// is an implementation detail. The important part of this unit test is that
+// they are batched if it's possible that they happened in the same event.
 expect(ReactNoop).toFlushAndYield([
 'A [render]',
 'A [commit]',
@@ -138,21 +165,15 @@ describe('ReactExpiration', () => {
 // Now do the same thing again, except this time don't flush any work in
 // between the two updates.
 ReactNoop.render(<Texttext="A"/>);
-// Advance the timer, but don't flush the expired work. Because we still
-// haven't entered an idle callback, the scheduler must assume that we're
-// inside the same event.
-ReactNoop.advanceTime(2000);
+Scheduler.advanceTime(2000);
 expect(ReactNoop).toHaveYielded([]);
 expect(ReactNoop.getChildren()).toEqual([span('B')]);
 
-// Perform some synchronous work. Again, the scheduler must assume we're
-// inside the same event.
-ReactNoop.flushSync(()=>{
-ReactNoop.renderToRootWithID('1','second-root');
-});
+// Perform some synchronous work. The scheduler must assume we're inside
+// the same event.
+interrupt();
 
-// Even though React flushed a sync update, it should not have updated the
-// current time. Schedule another update.
+// Schedule another update.
 ReactNoop.render(<Texttext="B"/>);
 // The updates should flush in the same batch, since as far as the scheduler
 // knows, they may have occurred inside the same event.