Rename variables to remove references to 'global' global

[flarnie]

what is the change?:
In a recent PR we were referencing some global variables and storing
local references to them.

To make things more natural, we co-opted the original name of the global
for our local reference. To make this work with Flow, we get the
original reference from 'window.requestAnimationFrame' and assign it to
'const requestAnimationFrame'.

Sometimes React is used in an environment where 'window' is not defined

• in that case we need to use something else, or hide the 'window'
  reference somewhere.

We opted to use 'global' thinking that Babel transforms would fill that
in with the proper thing.

But for some of our fixtures we are not doing that transform on the
bundle.

why make this change?:
I want to unbreak this on master and then investigate more about what we
should do to fix this.

Would like to land this quick fix but ultimately would like to avoid using the confusing 'localRequestAnimationFrame' naming.

test plan:
run yarn build and open the fixtures.

issue:

fixes #12930

#12930

[gaearon]

If it fails for fixtures I think it means it would fail after the release.

I don’t recall us using any transform that would turn global into a polyfill. So it’s probably just global in the output (which I would expect to break in any browser). JSDOM tests wouldn’t catch this so that’s why we missed it.

I’m not sure why ESLint checks on UMD bundles didn’t catch this. We have a whitelist if allowed globals. It’s worth following up to look at that. I guess maybe node environment is whitelisted there for the sake of UMD wrapper itself.

[flarnie]

Thanks. I think there is something I'm missing - will look into it, but I was also suprised the build tests didn't catch this.

[sompylasar]

@flarnie @gaearon This change seems to have broken the server-side rendering: I'm attempting to yarn start the fixtures/ssr to test #12063 and getting the following error from the server-side when I load the localhost:3000 page:

ReferenceError: requestAnimationFrame is not defined
    at /__CENSORED__/react/build/node_modules/react-dom/cjs/react-dom.development.js:5232:34
    at Object.<anonymous> (/__CENSORED__/react/build/node_modules/react-dom/cjs/react-dom.development.js:17632:5)
    at Module._compile (module.js:624:30)
    at Module._extensions..js (module.js:635:10)
    at Object.require.extensions.(anonymous function) [as .js] (/__CENSORED__/react/fixtures/ssr/node_modules/babel-register/lib/node.js:152:7)
    at Module.load (module.js:545:32)
    at tryModuleLoad (module.js:508:12)
    at Function.Module._load (module.js:500:3)
    at Module.require (module.js:568:17)
    at require (internal/module.js:11:18)

It points to this line:

// 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.
const localRequestAnimationFrame = requestAnimationFrame;

[gaearon]

@sompylasar

Can you please send a fix? I imagine we’d want to only do this in a DOM environment. There should be an environment check later in that file.

[sompylasar]

@gaearon Something like:

const localRequestAnimationFrame = (
ExecutionEnvironment.canUseDOM &&
    typeof requestAnimationFrame === 'function'
    ? requestAnimationFrame
    : null
);

or something else? What do you want localRequestAnimationFrame to be if requestAnimationFrame is missing or invalid?

[gaearon]

Let's just export whatever it is equal to without checks. We check at the call site, and this module is temporary so it would be nice to keep it simple.

const localRequestAnimationFrame =
  typeof requestAnimationFrame === 'function'
    ? requestAnimationFrame
    : undefined;

[sompylasar]

Ok, undefined it will be then.

[sompylasar]

👉 #13000 fixed in #13001