Skip to content

Commit

Permalink
Temporary fix for grabbing wrong rAF polyfill in ReactScheduler (face…
Browse files Browse the repository at this point in the history
…book#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
  • Loading branch information
flarnie authored May 17, 2018
1 parent 4b8510b commit 7ccb371
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 16 deletions.
19 changes: 3 additions & 16 deletions packages/react-scheduler/src/ReactScheduler.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -232,7 +219,7 @@ if (!ExecutionEnvironment.canUseDOM) {
if (!isAnimationFrameScheduled) {
// Schedule another animation callback so we retry later.
isAnimationFrameScheduled = true;
requestAnimationFrame(animationTick);
requestAnimationFrameForReact(animationTick);
}
}
};
Expand Down Expand Up @@ -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;
};
Expand Down
10 changes: 10 additions & 0 deletions packages/shared/forks/requestAnimationFrameForReact.www.js
Original file line number Diff line number Diff line change
@@ -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');
28 changes: 28 additions & 0 deletions packages/shared/requestAnimationFrameForReact.js
Original file line number Diff line number Diff line change
@@ -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;
11 changes: 11 additions & 0 deletions scripts/rollup/forks.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit 7ccb371

Please sign in to comment.