Skip to content

Fix ReferenceError when requestAnimationFrame isn't defined #13152

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 5, 2018
Merged

Fix ReferenceError when requestAnimationFrame isn't defined #13152

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d3e762f
Make the test fail
gaearon Jul 5, 2018
9064cf5
Fix rAF detection to avoid a ReferenceError
gaearon Jul 5, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/react-dom/src/__tests__/ReactDOM-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -434,10 +434,10 @@ describe('ReactDOM', () => {
}
});

it('warns when requestAnimationFrame is not polyfilled in the browser', () => {
it('warns when requestAnimationFrame is not polyfilled', () => {
const previousRAF = global.requestAnimationFrame;
try {
global.requestAnimationFrame = undefined;
delete global.requestAnimationFrame;
jest.resetModules();
expect(() => require('react-dom')).toWarnDev(
"This browser doesn't support requestAnimationFrame.",
Expand Down
42 changes: 28 additions & 14 deletions packages/react-scheduler/src/ReactScheduler.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,18 +44,6 @@ export type CallbackIdType = CallbackConfigType;
import {canUseDOM} from 'shared/ExecutionEnvironment';
import warning from 'shared/warning';

if (__DEV__) {
if (canUseDOM && typeof requestAnimationFrame !== 'function') {
warning(
false,
// TODO: reword this when schedule is a stand-alone module
"This browser doesn't support requestAnimationFrame. " +
'Make sure that you load a ' +
'polyfill in older browsers. https://fb.me/react-polyfills',
);
}
}

// We capture a local reference to any global, in case it gets polyfilled after
// this module is initially evaluated.
// We want to be using a consistent implementation.
Expand All @@ -64,12 +52,24 @@ const localDate = Date;
// This initialization code may run even on server environments
// if a component just imports ReactDOM (e.g. for findDOMNode).
// Some environments might not have setTimeout or clearTimeout.
// However, we always expect them to be defined on the client.
// https://github.com/facebook/react/pull/13088
const localSetTimeout =
typeof setTimeout === 'function' ? setTimeout : (undefined: any);
const localClearTimeout =
typeof clearTimeout === 'function' ? clearTimeout : (undefined: any);

// We don't expect either of these to necessarily be defined,
// but we will error later if they are missing on the client.
const localRequestAnimationFrame =
typeof requestAnimationFrame === 'function'
? requestAnimationFrame
: (undefined: any);
const localCancelAnimationFrame =
typeof cancelAnimationFrame === 'function'
? cancelAnimationFrame
: (undefined: any);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This module throws if this is undefined, right? Maybe we should use an invariant?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't want to spend the bytes.

I think it actually used an invariant in the past, and Sebastian wanted to relax that to a warning.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess in sync mode this module is never used so an invariant would be harsh


const hasNativePerformanceNow =
typeof performance === 'object' && typeof performance.now === 'function';

Expand Down Expand Up @@ -123,8 +123,22 @@ if (!canUseDOM) {
localClearTimeout(timeoutId);
};
} else {
const localRequestAnimationFrame = requestAnimationFrame;
const localCancelAnimationFrame = cancelAnimationFrame;
if (typeof localRequestAnimationFrame !== 'function') {
warning(
false,
"This browser doesn't support requestAnimationFrame. " +
'Make sure that you load a ' +
'polyfill in older browsers. https://fb.me/react-polyfills',
);
}
if (typeof localCancelAnimationFrame !== 'function') {
warning(
false,
"This browser doesn't support cancelAnimationFrame. " +
'Make sure that you load a ' +
'polyfill in older browsers. https://fb.me/react-polyfills',
);
}

let headOfPendingCallbacksLinkedList: CallbackConfigType | null = null;
let tailOfPendingCallbacksLinkedList: CallbackConfigType | null = null;
Expand Down