From 7ccb37161ffcbea33d6ede1397098faa567751f3 Mon Sep 17 00:00:00 2001 From: Flarnie Marchan Date: Thu, 17 May 2018 08:57:45 -0700 Subject: [PATCH] Temporary fix for grabbing wrong rAF polyfill in ReactScheduler (#12837) * Temporary fix for grabbing wrong rAF polyfill in ReactScheduler **what is the change?:** For now... We need to grab a slightly different implementation of rAF internally at FB than in Open Source. Making rAF a dependency of the ReactScheduler module allows us to fork the dependency at FB. NOTE: After this lands we have an alternative plan to make this module separate from React and require it before our Facebook timer polyfills are applied. But want to land this now to keep master in a working state and fix bugs folks are seeing at Facebook. Thanks @sebmarkbage @acdlite and @sophiebits for discussing the options and trade-offs for solving this issue. **why make this change?:** This fixes a problem we're running into when experimenting with ReactScheduler internally at Facebook, **and* it's part of our long term plan to use dependency injection with the scheduler to make it easier to test and adjust. **test plan:** Ran tests, lint, flow, and will manually test when syncing into Facebook's codebase. **issue:** See internal task T29442940 * ran prettier --- .../react-scheduler/src/ReactScheduler.js | 19 ++----------- .../requestAnimationFrameForReact.www.js | 10 +++++++ .../shared/requestAnimationFrameForReact.js | 28 +++++++++++++++++++ scripts/rollup/forks.js | 11 ++++++++ 4 files changed, 52 insertions(+), 16 deletions(-) create mode 100644 packages/shared/forks/requestAnimationFrameForReact.www.js create mode 100644 packages/shared/requestAnimationFrameForReact.js diff --git a/packages/react-scheduler/src/ReactScheduler.js b/packages/react-scheduler/src/ReactScheduler.js index 045612bceb..942d69de9c 100644 --- a/packages/react-scheduler/src/ReactScheduler.js +++ b/packages/react-scheduler/src/ReactScheduler.js @@ -39,20 +39,7 @@ type CallbackConfigType = {| |}; import ExecutionEnvironment from 'fbjs/lib/ExecutionEnvironment'; -import warning from 'fbjs/lib/warning'; - -if (__DEV__) { - if ( - ExecutionEnvironment.canUseDOM && - typeof requestAnimationFrame !== 'function' - ) { - warning( - false, - 'React depends on requestAnimationFrame. Make sure that you load a ' + - 'polyfill in older browsers. https://fb.me/react-polyfills', - ); - } -} +import requestAnimationFrameForReact from 'shared/requestAnimationFrameForReact'; const hasNativePerformanceNow = typeof performance === 'object' && typeof performance.now === 'function'; @@ -232,7 +219,7 @@ if (!ExecutionEnvironment.canUseDOM) { if (!isAnimationFrameScheduled) { // Schedule another animation callback so we retry later. isAnimationFrameScheduled = true; - requestAnimationFrame(animationTick); + requestAnimationFrameForReact(animationTick); } } }; @@ -298,7 +285,7 @@ if (!ExecutionEnvironment.canUseDOM) { // might want to still have setTimeout trigger scheduleWork as a backup to ensure // that we keep performing work. isAnimationFrameScheduled = true; - requestAnimationFrame(animationTick); + requestAnimationFrameForReact(animationTick); } return newCallbackId; }; diff --git a/packages/shared/forks/requestAnimationFrameForReact.www.js b/packages/shared/forks/requestAnimationFrameForReact.www.js new file mode 100644 index 0000000000..f082644eca --- /dev/null +++ b/packages/shared/forks/requestAnimationFrameForReact.www.js @@ -0,0 +1,10 @@ +/** + * Copyright (c) 2013-present, Facebook, Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +'use strict'; + +export default require('requestAnimationFrameForReact'); diff --git a/packages/shared/requestAnimationFrameForReact.js b/packages/shared/requestAnimationFrameForReact.js new file mode 100644 index 0000000000..aa59610322 --- /dev/null +++ b/packages/shared/requestAnimationFrameForReact.js @@ -0,0 +1,28 @@ +/** + * Copyright (c) 2013-present, Facebook, Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ + +'use strict'; + +import ExecutionEnvironment from 'fbjs/lib/ExecutionEnvironment'; +import warning from 'fbjs/lib/warning'; + +if (__DEV__) { + if ( + ExecutionEnvironment.canUseDOM && + typeof requestAnimationFrame !== 'function' + ) { + warning( + false, + 'React depends on requestAnimationFrame. Make sure that you load a ' + + 'polyfill in older browsers. https://fb.me/react-polyfills', + ); + } +} + +export default requestAnimationFrame; diff --git a/scripts/rollup/forks.js b/scripts/rollup/forks.js index 28de8651c1..7df244733f 100644 --- a/scripts/rollup/forks.js +++ b/scripts/rollup/forks.js @@ -74,6 +74,17 @@ const forks = Object.freeze({ return null; }, + // This logic is forked on www to use the 'acrossTransitions' version. + 'shared/requestAnimationFrameForReact': (bundleType, entry) => { + switch (bundleType) { + case FB_WWW_DEV: + case FB_WWW_PROD: + return 'shared/forks/requestAnimationFrameForReact.www.js'; + default: + return null; + } + }, + // This logic is forked on www to blacklist warnings. 'shared/lowPriorityWarning': (bundleType, entry) => { switch (bundleType) {