@samitouri / QOS-React / commits / 1e3cd332a0

Remove the 'alwaysUseRequestIdleCallbackPolyfill' feature flag (#12648)

* Remove the 'alwaysUseRequestIdleCallbackPolyfill' feature flag **what is the change?:** Removes the feature flag 'alwaysUseRequestIdleCallbackPolyfill', such that we **always** use the polyfill for requestIdleCallback. **why make this change?:** We have been testing this feature flag at 100% for some time internally, and determined it works better for React than the native implementation. Looks like RN was overriding the flag to use the native when possible, but since no RN products are using 'async' mode it should be safe to switch this flag over for RN as well. **test plan:** We have already been testing this internally for some time. **issue:** internal task t28128480 * fix mistaken conditional * Add mocking of rAF, postMessage, and initial test for ReactScheduler **what is the change?:** - In all tests where we previously mocked rIC or relied on native mocking which no longer works, we are now mocking rAF and postMessage. - Also adds a basic initial test for ReactScheduler. NOTE -> we do plan to write headless browser tests for ReactScheduler! This is just an initial test, to verify that it works with the mocked out browser APIs as expected. **why make this change?:** We need to mock out the browser APIs more completely for the new 'ReactScheduler' to work in our tests. Many tests are depending on it, since it's used at a low level. By mocking the browser APIs rather than the 'react-scheduler' module, we enable testing the production bundles. This approach is trading isolation for accuracy. These tests will be closer to a real use. **test plan:** run the tests :) **issue:** internal task T28128480

Flarnie Marchan committed Apr 23, 2018 at 15:25 UTC 1e3cd332a015e312149efa36eb81c7523411cc2d
15 files changed +194 -70
packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.internal.js
+25
@@ -20,6 +20,31 @@ describe('ReactDOMFiberAsync', () => {
20 let container;
21
22 beforeEach(() => {
23 + // TODO pull this into helper method, reduce repetition.
24 + // mock the browser APIs which are used in react-scheduler:
25 + // - requestAnimationFrame should pass the DOMHighResTimeStamp argument
26 + // - calling 'window.postMessage' should actually fire postmessage handlers
27 + global.requestAnimationFrame = function(cb) {
28 + return setTimeout(() => {
29 + cb(Date.now());
30 + });
31 + };
32 + const originalAddEventListener = global.addEventListener;
33 + let postMessageCallback;
34 + global.addEventListener = function(eventName, callback, useCapture) {
35 + if (eventName === 'message') {
36 + postMessageCallback = callback;
37 + } else {
38 + originalAddEventListener(eventName, callback, useCapture);
39 + }
40 + };
41 + global.postMessage = function(messageKey, targetOrigin) {
42 + const postMessageEvent = {source: window, data: messageKey};
43 + if (postMessageCallback) {
44 + postMessageCallback(postMessageEvent);
45 + }
46 + };
47 + jest.resetModules();
48 container = document.createElement('div');
49 ReactDOM = require('react-dom');
50 });
packages/react-dom/src/__tests__/ReactDOMRoot-test.js
+50 -51
@@ -17,47 +17,46 @@ let AsyncMode = React.unstable_AsyncMode;
17 describe('ReactDOMRoot', () => {
18 let container;
19
20 - let scheduledCallback;
21 - let flush;
22 - let now;
23 - let expire;
20 + let advanceCurrentTime;
21
22 beforeEach(() => {
23 container = document.createElement('div');
27 -
28 - // Override requestIdleCallback
29 - scheduledCallback = null;
30 - flush = function(units = Infinity) {
31 - if (scheduledCallback !== null) {
32 - let didStop = false;
33 - while (scheduledCallback !== null && !didStop) {
34 - const cb = scheduledCallback;
35 - scheduledCallback = null;
36 - cb({
37 - timeRemaining() {
38 - if (units > 0) {
39 - return 999;
40 - }
41 - didStop = true;
42 - return 0;
43 - },
44 - });
45 - units--;
46 - }
24 + // TODO pull this into helper method, reduce repetition.
25 + // mock the browser APIs which are used in react-scheduler:
26 + // - requestAnimationFrame should pass the DOMHighResTimeStamp argument
27 + // - calling 'window.postMessage' should actually fire postmessage handlers
28 + // - must allow artificially changing time returned by Date.now
29 + // Performance.now is not supported in the test environment
30 + const originalDateNow = Date.now;
31 + let advancedTime = null;
32 + global.Date.now = function() {
33 + if (advancedTime) {
34 + return originalDateNow() + advancedTime;
35 }
36 + return originalDateNow();
37 };
49 - global.performance = {
50 - now() {
51 - return now;
52 - },
38 + advanceCurrentTime = function(amount) {
39 + advancedTime = amount;
40 };
54 - global.requestIdleCallback = function(cb) {
55 - scheduledCallback = cb;
41 + global.requestAnimationFrame = function(cb) {
42 + return setTimeout(() => {
43 + cb(Date.now());
44 + });
45 };
57 -
58 - now = 0;
59 - expire = function(ms) {
60 - now += ms;
46 + const originalAddEventListener = global.addEventListener;
47 + let postMessageCallback;
48 + global.addEventListener = function(eventName, callback, useCapture) {
49 + if (eventName === 'message') {
50 + postMessageCallback = callback;
51 + } else {
52 + originalAddEventListener(eventName, callback, useCapture);
53 + }
54 + };
55 + global.postMessage = function(messageKey, targetOrigin) {
56 + const postMessageEvent = {source: window, data: messageKey};
57 + if (postMessageCallback) {
58 + postMessageCallback(postMessageEvent);
59 + }
60 };
61
62 jest.resetModules();
@@ -70,17 +69,17 @@ describe('ReactDOMRoot', () => {
69 it('renders children', () => {
70 const root = ReactDOM.unstable_createRoot(container);
71 root.render(<div>Hi</div>);
73 - flush();
72 + jest.runAllTimers();
73 expect(container.textContent).toEqual('Hi');
74 });
75
76 it('unmounts children', () => {
77 const root = ReactDOM.unstable_createRoot(container);
78 root.render(<div>Hi</div>);
80 - flush();
79 + jest.runAllTimers();
80 expect(container.textContent).toEqual('Hi');
81 root.unmount();
83 - flush();
82 + jest.runAllTimers();
83 expect(container.textContent).toEqual('');
84 });
85
@@ -92,7 +91,7 @@ describe('ReactDOMRoot', () => {
91 ops.push('inside callback: ' + container.textContent);
92 });
93 ops.push('before committing: ' + container.textContent);
95 - flush();
94 + jest.runAllTimers();
95 ops.push('after committing: ' + container.textContent);
96 expect(ops).toEqual([
97 'before committing: ',
@@ -105,7 +104,7 @@ describe('ReactDOMRoot', () => {
104 it('resolves `work.then` callback synchronously if the work already committed', () => {
105 const root = ReactDOM.unstable_createRoot(container);
106 const work = root.render(<AsyncMode>Hi</AsyncMode>);
108 - flush();
107 + jest.runAllTimers();
108 let ops = [];
109 work.then(() => {
110 ops.push('inside callback');
@@ -133,7 +132,7 @@ describe('ReactDOMRoot', () => {
132 <span />
133 </div>,
134 );
136 - flush();
135 + jest.runAllTimers();
136
137 // Accepts `hydrate` option
138 const container2 = document.createElement('div');
@@ -144,7 +143,7 @@ describe('ReactDOMRoot', () => {
143 <span />
144 </div>,
145 );
147 - expect(flush).toWarnDev('Extra attributes');
146 + expect(jest.runAllTimers).toWarnDev('Extra attributes');
147 });
148
149 it('does not clear existing children', async () => {
@@ -156,7 +155,7 @@ describe('ReactDOMRoot', () => {
155 <span>d</span>
156 </div>,
157 );
159 - flush();
158 + jest.runAllTimers();
159 expect(container.textContent).toEqual('abcd');
160 root.render(
161 <div>
@@ -164,7 +163,7 @@ describe('ReactDOMRoot', () => {
163 <span>c</span>
164 </div>,
165 );
167 - flush();
166 + jest.runAllTimers();
167 expect(container.textContent).toEqual('abdc');
168 });
169
@@ -200,7 +199,7 @@ describe('ReactDOMRoot', () => {
199 </AsyncMode>,
200 );
201
203 - flush();
202 + jest.runAllTimers();
203
204 // Hasn't updated yet
205 expect(container.textContent).toEqual('');
@@ -229,7 +228,7 @@ describe('ReactDOMRoot', () => {
228 const batch = root.createBatch();
229 batch.render(<Foo>Hi</Foo>);
230 // Flush all async work.
232 - flush();
231 + jest.runAllTimers();
232 // Root should complete without committing.
233 expect(ops).toEqual(['Foo']);
234 expect(container.textContent).toEqual('');
@@ -247,7 +246,7 @@ describe('ReactDOMRoot', () => {
246 const batch = root.createBatch();
247 batch.render(<AsyncMode>Foo</AsyncMode>);
248
250 - flush();
249 + jest.runAllTimers();
250
251 // Hasn't updated yet
252 expect(container.textContent).toEqual('');
@@ -287,7 +286,7 @@ describe('ReactDOMRoot', () => {
286 const root = ReactDOM.unstable_createRoot(container);
287 root.render(<AsyncMode>1</AsyncMode>);
288
290 - expire(2000);
289 + advanceCurrentTime(2000);
290 // This batch has a later expiration time than the earlier update.
291 const batch = root.createBatch();
292
@@ -295,7 +294,7 @@ describe('ReactDOMRoot', () => {
294 batch.commit();
295 expect(container.textContent).toEqual('');
296
298 - flush();
297 + jest.runAllTimers();
298 expect(container.textContent).toEqual('1');
299 });
300
@@ -322,7 +321,7 @@ describe('ReactDOMRoot', () => {
321 batch1.render(1);
322
323 // This batch has a later expiration time
325 - expire(2000);
324 + advanceCurrentTime(2000);
325 const batch2 = root.createBatch();
326 batch2.render(2);
327
@@ -341,7 +340,7 @@ describe('ReactDOMRoot', () => {
340 batch1.render(1);
341
342 // This batch has a later expiration time
344 - expire(2000);
343 + advanceCurrentTime(2000);
344 const batch2 = root.createBatch();
345 batch2.render(2);
346
@@ -351,7 +350,7 @@ describe('ReactDOMRoot', () => {
350 expect(container.textContent).toEqual('2');
351
352 batch1.commit();
354 - flush();
353 + jest.runAllTimers();
354 expect(container.textContent).toEqual('1');
355 });
356
packages/react-dom/src/events/__tests__/ChangeEventPlugin-test.internal.js
+25
@@ -32,6 +32,31 @@ describe('ChangeEventPlugin', () => {
32 let container;
33
34 beforeEach(() => {
35 + // TODO pull this into helper method, reduce repetition.
36 + // mock the browser APIs which are used in react-scheduler:
37 + // - requestAnimationFrame should pass the DOMHighResTimeStamp argument
38 + // - calling 'window.postMessage' should actually fire postmessage handlers
39 + global.requestAnimationFrame = function(cb) {
40 + return setTimeout(() => {
41 + cb(Date.now());
42 + });
43 + };
44 + const originalAddEventListener = global.addEventListener;
45 + let postMessageCallback;
46 + global.addEventListener = function(eventName, callback, useCapture) {
47 + if (eventName === 'message') {
48 + postMessageCallback = callback;
49 + } else {
50 + originalAddEventListener(eventName, callback, useCapture);
51 + }
52 + };
53 + global.postMessage = function(messageKey, targetOrigin) {
54 + const postMessageEvent = {source: window, data: messageKey};
55 + if (postMessageCallback) {
56 + postMessageCallback(postMessageEvent);
57 + }
58 + };
59 + jest.resetModules();
60 container = document.createElement('div');
61 document.body.appendChild(container);
62 });
packages/react-dom/src/events/__tests__/SimpleEventPlugin-test.internal.js
+25
@@ -33,6 +33,31 @@ describe('SimpleEventPlugin', function() {
33 }
34
35 beforeEach(function() {
36 + // TODO pull this into helper method, reduce repetition.
37 + // mock the browser APIs which are used in react-scheduler:
38 + // - requestAnimationFrame should pass the DOMHighResTimeStamp argument
39 + // - calling 'window.postMessage' should actually fire postmessage handlers
40 + global.requestAnimationFrame = function(cb) {
41 + return setTimeout(() => {
42 + cb(Date.now());
43 + });
44 + };
45 + const originalAddEventListener = global.addEventListener;
46 + let postMessageCallback;
47 + global.addEventListener = function(eventName, callback, useCapture) {
48 + if (eventName === 'message') {
49 + postMessageCallback = callback;
50 + } else {
51 + originalAddEventListener(eventName, callback, useCapture);
52 + }
53 + };
54 + global.postMessage = function(messageKey, targetOrigin) {
55 + const postMessageEvent = {source: window, data: messageKey};
56 + if (postMessageCallback) {
57 + postMessageCallback(postMessageEvent);
58 + }
59 + };
60 + jest.resetModules();
61 React = require('react');
62 ReactDOM = require('react-dom');
63 ReactTestUtils = require('react-dom/test-utils');
packages/react-scheduler/src/ReactScheduler.js
+3 -10
@@ -32,7 +32,6 @@
32
33 import type {Deadline} from 'react-reconciler';
34
35 -import {alwaysUseRequestIdleCallbackPolyfill} from 'shared/ReactFeatureFlags';
35 import ExecutionEnvironment from 'fbjs/lib/ExecutionEnvironment';
36 import warning from 'fbjs/lib/warning';
37
@@ -85,12 +84,8 @@ if (!ExecutionEnvironment.canUseDOM) {
84 cIC = function(timeoutID: number) {
85 clearTimeout(timeoutID);
86 };
88 -} else if (
89 - alwaysUseRequestIdleCallbackPolyfill ||
90 - typeof requestIdleCallback !== 'function' ||
91 - typeof cancelIdleCallback !== 'function'
92 -) {
93 - // Polyfill requestIdleCallback and cancelIdleCallback
87 +} else {
88 + // Always polyfill requestIdleCallback and cancelIdleCallback
89
90 let scheduledRICCallback = null;
91 let isIdleScheduled = false;
@@ -175,6 +170,7 @@ if (!ExecutionEnvironment.canUseDOM) {
170 window.addEventListener('message', idleTick, false);
171
172 const animationTick = function(rafTime) {
173 + console.log('animationTick called and rafTime is ', rafTime);
174 isAnimationFrameScheduled = false;
175 let nextFrameTime = rafTime - frameDeadline + activeFrameTime;
176 if (
@@ -231,9 +227,6 @@ if (!ExecutionEnvironment.canUseDOM) {
227 isIdleScheduled = false;
228 timeoutTime = -1;
229 };
234 -} else {
235 - rIC = window.requestIdleCallback;
236 - cIC = window.cancelIdleCallback;
230 }
231
232 export {now, rIC, cIC};
packages/react-scheduler/src/__tests__/ReactScheduler-test.js new
+56
@@ -0,0 +1,56 @@
1 +/**
2 + * Copyright (c) 2013-present, Facebook, Inc.
3 + *
4 + * This source code is licensed under the MIT license found in the
5 + * LICENSE file in the root directory of this source tree.
6 + *
7 + * @emails react-core
8 + */
9 +
10 +'use strict';
11 +
12 +let ReactScheduler;
13 +
14 +describe('ReactScheduler', () => {
15 + beforeEach(() => {
16 + // TODO pull this into helper method, reduce repetition.
17 + // mock the browser APIs which are used in react-scheduler:
18 + // - requestAnimationFrame should pass the DOMHighResTimeStamp argument
19 + // - calling 'window.postMessage' should actually fire postmessage handlers
20 + global.requestAnimationFrame = function(cb) {
21 + return setTimeout(() => {
22 + cb(Date.now());
23 + });
24 + };
25 + const originalAddEventListener = global.addEventListener;
26 + let postMessageCallback;
27 + global.addEventListener = function(eventName, callback, useCapture) {
28 + if (eventName === 'message') {
29 + postMessageCallback = callback;
30 + } else {
31 + originalAddEventListener(eventName, callback, useCapture);
32 + }
33 + };
34 + global.postMessage = function(messageKey, targetOrigin) {
35 + const postMessageEvent = {source: window, data: messageKey};
36 + if (postMessageCallback) {
37 + postMessageCallback(postMessageEvent);
38 + }
39 + };
40 + jest.resetModules();
41 + ReactScheduler = require('react-scheduler');
42 + });
43 +
44 + it('rIC calls the callback within the frame when not blocked', () => {
45 + const {rIC} = ReactScheduler;
46 + const cb = jest.fn();
47 + rIC(cb);
48 + jest.runAllTimers();
49 + expect(cb.mock.calls.length).toBe(1);
50 + // should have ... TODO details on what we expect
51 + expect(cb.mock.calls[0][0].didTimeout).toBe(false);
52 + expect(typeof cb.mock.calls[0][0].timeRemaining()).toBe('number');
53 + });
54 +
55 + // TODO: test cIC and now
56 +});
packages/react-scheduler/src/__tests__/test_page.html new
+10
@@ -0,0 +1,10 @@
1 +<!DOCTYPE html>
2 +<html style="width: 100%; height: 100%; overflow: hidden">
3 + <head>
4 + <meta charset="utf-8">
5 + <title>React Scheduler test page</title>
6 + </head>
7 + <body>
8 + <h1>Hello World</h1>
9 + </body>
10 +</html>
packages/shared/ReactFeatureFlags.js
-2
@@ -37,8 +37,6 @@ export const replayFailedUnitOfWorkWithInvokeGuardedCallback = __DEV__;
37 // Warn about deprecated, async-unsafe lifecycles; relates to RFC #6:
38 export const warnAboutDeprecatedLifecycles = false;
39
40 -export const alwaysUseRequestIdleCallbackPolyfill = false;
41 -
40 // Only used in www builds.
41 export function addUserTimingListener() {
42 invariant(false, 'Not implemented.');
packages/shared/forks/ReactFeatureFlags.native-fabric-fb.js
-1
@@ -23,7 +23,6 @@ export const replayFailedUnitOfWorkWithInvokeGuardedCallback = __DEV__;
23 export const enableMutatingReconciler = false;
24 export const enableNoopReconciler = false;
25 export const enablePersistentReconciler = true;
26 -export const alwaysUseRequestIdleCallbackPolyfill = false;
26
27 // Only used in www builds.
28 export function addUserTimingListener() {
packages/shared/forks/ReactFeatureFlags.native-fabric-oss.js
-1
@@ -23,7 +23,6 @@ export const replayFailedUnitOfWorkWithInvokeGuardedCallback = __DEV__;
23 export const enableMutatingReconciler = false;
24 export const enableNoopReconciler = false;
25 export const enablePersistentReconciler = true;
26 -export const alwaysUseRequestIdleCallbackPolyfill = false;
26
27 // Only used in www builds.
28 export function addUserTimingListener() {
packages/shared/forks/ReactFeatureFlags.native-fb.js
-1
@@ -26,7 +26,6 @@ export const enableUserTimingAPI = __DEV__;
26 export const enableMutatingReconciler = true;
27 export const enableNoopReconciler = false;
28 export const enablePersistentReconciler = false;
29 -export const alwaysUseRequestIdleCallbackPolyfill = false;
29
30 // Only used in www builds.
31 export function addUserTimingListener() {
packages/shared/forks/ReactFeatureFlags.native-oss.js
-1
@@ -12,7 +12,6 @@ import invariant from 'fbjs/lib/invariant';
12 import typeof * as FeatureFlagsType from 'shared/ReactFeatureFlags';
13 import typeof * as FeatureFlagsShimType from './ReactFeatureFlags.native-oss';
14
15 -export const alwaysUseRequestIdleCallbackPolyfill = false;
15 export const debugRenderPhaseSideEffects = false;
16 export const debugRenderPhaseSideEffectsForStrictMode = false;
17 export const enableGetDerivedStateFromCatch = false;
packages/shared/forks/ReactFeatureFlags.persistent.js
-1
@@ -24,7 +24,6 @@ export const replayFailedUnitOfWorkWithInvokeGuardedCallback = __DEV__;
24 export const enableMutatingReconciler = false;
25 export const enableNoopReconciler = false;
26 export const enablePersistentReconciler = true;
27 -export const alwaysUseRequestIdleCallbackPolyfill = false;
27
28 // Only used in www builds.
29 export function addUserTimingListener() {
packages/shared/forks/ReactFeatureFlags.test-renderer.js
-1
@@ -21,7 +21,6 @@ export const replayFailedUnitOfWorkWithInvokeGuardedCallback = false;
21 export const enableMutatingReconciler = true;
22 export const enableNoopReconciler = false;
23 export const enablePersistentReconciler = false;
24 -export const alwaysUseRequestIdleCallbackPolyfill = false;
24
25 // Only used in www builds.
26 export function addUserTimingListener() {
packages/shared/forks/ReactFeatureFlags.www.js
-1
@@ -17,7 +17,6 @@ export const {
17 debugRenderPhaseSideEffectsForStrictMode,
18 warnAboutDeprecatedLifecycles,
19 replayFailedUnitOfWorkWithInvokeGuardedCallback,
20 - alwaysUseRequestIdleCallbackPolyfill,
20 } = require('ReactFeatureFlags');
21
22 // The rest of the flags are static for better dead code elimination.