Skip to content

Commit

Permalink
Fix polyfilling of regeneratorRuntime to avoid setting it to undefine…
Browse files Browse the repository at this point in the history
…d in some situations

Summary:
This diff fixes an issue that caused the problem with `regeneratorRuntime` last Friday (more info: https://fb.facebook.com/groups/frontendsupport/permalink/2427883350560427/).

The root issue is that both `InitializeCore` and `FBInitializeCore` are included in the same bundle, this fix just prevents the bundle from being invalid once this happens..

*copied from: https://our.intern.facebook.com/intern/diff/D10444264/?transaction_id=595485237532887*

The way that `regeneratorRuntime` is polyfilled is not correct:

```
polyfillGlobal('regeneratorRuntime', () => {
  // The require just sets up the global, so make sure when we first
  // invoke it the global does not exist
  delete global.regeneratorRuntime;
  require('regenerator-runtime/runtime');
  return global.regeneratorRuntime;
});
```

Since a `require`d module is only evaluated once (no matter how many times it's required), defining (and calling) a getter for `global.regeneratorRuntime` twice will end up storing `undefined` to `global.regeneratorRuntime`.

There were no issues before this diff because the ordering of requires made things work by coincidence, e.g the following code will work:

```
// Set up regenerator for the first time
polyfillGlobal('regeneratorRuntime', () => {
  delete global.regeneratorRuntime;
  require('regenerator-runtime/runtime');
  return global.regeneratorRuntime;
});

// Set up regenerator for the second time. This will just override the previous getter (which has not even got executed so
// the `regenerator-runtime/runtime` module has not been evaluated yet.
polyfillGlobal('regeneratorRuntime', () => {
  // The require just sets up the global, so make sure when we first
  // invoke it the global does not exist
  delete global.regeneratorRuntime;
  require('regenerator-runtime/runtime');
  return global.regeneratorRuntime;
});

// Now access regenerator
global.regeneratorRuntime;
```

But the following code won't work:

```
// Set up regenerator for the first time
polyfillGlobal('regeneratorRuntime', () => {
  delete global.regeneratorRuntime;
  require('regenerator-runtime/runtime');
  return global.regeneratorRuntime;
});

// Access regenerator. This will cause the previous getter to be called, and the `regenerator-runtime/runtime` module will get evaluated.
// Here, `global.regeneratorRuntime` will have a correct value.
global.regeneratorRuntime;

// Set up regenerator for the second time. This will define a new getter for `global.regeneratorRuntime`, which will delete `delete global.regeneratorRuntime`
// and return undefined (note that `require('regenerator-runtime/runtime');` is a noop since the module has been already evaluated).
polyfillGlobal('regeneratorRuntime', () => {
  // The require just sets up the global, so make sure when we first
  // invoke it the global does not exist
  delete global.regeneratorRuntime;
  require('regenerator-runtime/runtime');
  return global.regeneratorRuntime;
});
```

Reviewed By: fromcelticpark

Differential Revision: D10483975

fbshipit-source-id: 5b3ef6e11c4fc4f79e3857c1ade9e7bc2beb6a39
  • Loading branch information
rafeca authored and facebook-github-bot committed Oct 22, 2018
1 parent 0984196 commit 2a7e02e
Showing 1 changed file with 4 additions and 2 deletions.
6 changes: 4 additions & 2 deletions Libraries/Core/InitializeCore.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,10 @@ polyfillGlobal('regeneratorRuntime', () => {
// The require just sets up the global, so make sure when we first
// invoke it the global does not exist
delete global.regeneratorRuntime;
require('regenerator-runtime/runtime');
return global.regeneratorRuntime;

// regenerator-runtime/runtime exports the regeneratorRuntime object, so we
// can return it safely.
return require('regenerator-runtime/runtime');
});

// Set up timers
Expand Down

0 comments on commit 2a7e02e

Please sign in to comment.