Remove global babelHelpers and regenerator #21283

janicduplessis · 2018-09-24T03:39:51Z

Follow up to facebook/metro@8932a9c

Babel helpers and regenerator runtime will be imported automatically from @babel/runtime. We still need to add the global regeneratorRuntime for jest tests since we disable babel-runtime currently.

Test Plan:

Run RN tester and make sure it works fine for screens using async functions.
Inspect the generated bundle to make sure helpers and regenerator are imported from @babel/runtime
Made sure the global babelHelpers and the old regenerator-runtime are not in the generated bundle.
Run jest tests

Release Notes:

Help reviewers and the release process by writing your own release notes. See below for an example.

[GENERAL] [ENHANCEMENT] [polyfills] - Remove global babelHelpers and regenerator

…erator (#198) Summary: **Summary** The RN transformer currently relies on the enviroment providing babelHelpers and regeneratorRuntime as globals by using 'babel-external-helpers'. This wasn't really a problem before since helpers were stable and we could maintain our copy easily but it seems like there are more now with babel 7 and it makes sense to include only those used by the app. This is exactly what babel/transform-runtime does. It will alias all helpers and calls to regeneratorRuntime to files in the babel/runtime package. This will solve issues like this facebook/react-native#20150 caused by missing babelHelpers. This solution also avoids bloating babelHelpers to fix OSS issues like the one linked before. **Test plan** - Updated tests so they all pass. - Tested that it actually works by applying the changes locally in an RN app. - Added a test for async functions, to make sure regenerator is aliased properly and doesn't depend on the global. - Made sure require-test.js still fails if the require implementation contains babel helpers (by adding an empty class in the file). Pull Request resolved: #198 Reviewed By: mjesun Differential Revision: D8833903 Pulled By: rafeca fbshipit-source-id: 7081f769f288ab358ba89ae8ee72a513bb12e225

rafeca · 2018-09-24T12:37:14Z

Thanks @janicduplessis for the PR! I'm planning to release a new version of Metro later today (it's going to be v0.46.0 since the babel runtime change is breaking.

With this you're going to be able to test it locally and afterwards I can import it 😄

rafeca · 2018-09-24T16:23:45Z

I've just published metro@0.46.0 with the change. Can you test that things are working fine? In the meantime I'm importing the PR to do the internal checks

facebook-github-bot

rafeca has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

newyankeecodeshop · 2018-09-24T16:40:23Z

Libraries/Core/InitializeCore.js

-  /* $FlowFixMe(>=0.54.0 site=react_native_oss) This comment suppresses an
-   * error found when Flow v0.54 was deployed. To see the error delete this
-   * comment and run Flow. */
-  require('regenerator-runtime/runtime');


Can you also remove regenerator-runtime from the dependencies in package.json?

We still need it for jest tests, I guess I could move it to dev deps

janicduplessis · 2018-09-24T18:00:45Z

@rafeca I was able to tests it, the only change missing was to add @babel/runtime to RN deps.

janicduplessis · 2018-09-24T18:03:19Z

@rafeca I also didn't update the yarn.lock file, for some reason I couldn't generate a clean set of changes to it. It might be out of date already or a different version of yarn was used to generate it.

janicduplessis · 2018-09-24T18:09:29Z

Also CI seems to be failling because of metro package that isn't published properly

error Couldn't find any versions for "metro-cache" that matches "0.46.0"
info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.
Error: Couldn't find any versions for "metro-cache" that matches "0.46.0"
    at MessageError.ExtendableBuiltin (/opt/yarn-v1.9.4/lib/cli.js:243:66)
    at new MessageError (/opt/yarn-v1.9.4/lib/cli.js:272:123)
    at Function.<anonymous> (/opt/yarn-v1.9.4/lib/cli.js:51264:13)
    at Generator.next (<anonymous>)
    at step (/opt/yarn-v1.9.4/lib/cli.js:92:30)
    at /opt/yarn-v1.9.4/lib/cli.js:103:13
    at <anonymous>
    at process._tickCallback (internal/process/next_tick.js:189:7)
Exited with code 1

kelset · 2018-09-25T11:19:36Z

@janicduplessis I think it has been published now (or, at least, running npm view metro-cache says that 0.46.0 is out), could you re-trigger the CI (maybe via a new commit?).

facebook-github-bot

rafeca has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

janicduplessis · 2018-09-25T21:51:22Z

This might conflict with 1323acd, I think @babel/runtime should be a dependency and not devDependency.

kelset · 2018-09-26T08:20:19Z

Yeha I agree, may be worth reversing that in favour of this (which I guess may also just happen during the internal importing, right?)

cc @RSNara

react-native-bot · 2018-09-26T17:05:05Z

@janicduplessis merged commit 458d56c into facebook:master.

Once this commit is added to a release, you will see the corresponding version tag below the description at 458d56c. If the commit has a single master tag, it is not yet part of a release.

Summary: Follow up to facebook/metro@8932a9c Babel helpers and regenerator runtime will be imported automatically from `babel/runtime`. We still need to add the global regeneratorRuntime for jest tests since we disable babel-runtime currently. Pull Request resolved: #21283 Reviewed By: mjesun Differential Revision: D10010963 Pulled By: rafeca fbshipit-source-id: da5e23dd901f8f8940d46816b4fc9290d0e28c76

Summary: Follow up to facebook/metro@8932a9c Babel helpers and regenerator runtime will be imported automatically from `babel/runtime`. We still need to add the global regeneratorRuntime for jest tests since we disable babel-runtime currently. Pull Request resolved: facebook#21283 Reviewed By: mjesun Differential Revision: D10010963 Pulled By: rafeca fbshipit-source-id: da5e23dd901f8f8940d46816b4fc9290d0e28c76

Summary: Follow up to facebook/metro@8932a9c Babel helpers and regenerator runtime will be imported automatically from `babel/runtime`. We still need to add the global regeneratorRuntime for jest tests since we disable babel-runtime currently. Pull Request resolved: facebook/react-native#21283 Reviewed By: mjesun Differential Revision: D10010963 Pulled By: rafeca fbshipit-source-id: da5e23dd901f8f8940d46816b4fc9290d0e28c76

Summary: Follow up to facebook/metro@8932a9c Babel helpers and regenerator runtime will be imported automatically from `babel/runtime`. We still need to add the global regeneratorRuntime for jest tests since we disable babel-runtime currently. Pull Request resolved: facebook#21283 Reviewed By: mjesun Differential Revision: D10010963 Pulled By: rafeca fbshipit-source-id: da5e23dd901f8f8940d46816b4fc9290d0e28c76

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 24, 2018

This comment has been minimized.

Sign in to view

react-native-bot added Core Team labels Sep 24, 2018

facebook-github-bot reviewed Sep 24, 2018

View reviewed changes

newyankeecodeshop reviewed Sep 24, 2018

View reviewed changes

janicduplessis added 2 commits September 24, 2018 13:50

Remove global babelHelpers and regenerator

fc5f4a2

Add @babel/runtime dependency

d9f7588

janicduplessis force-pushed the remove-polyfills branch from b9798d6 to d9f7588 Compare September 24, 2018 17:57

janicduplessis requested a review from hramos as a code owner September 24, 2018 17:57

janicduplessis changed the title ~~[DONTMERGE] Remove global babelHelpers and regenerator~~ Remove global babelHelpers and regenerator Sep 24, 2018

Move regenerator to dev deps since its used only for tests

957c5b4

sunnylqm mentioned this pull request Sep 25, 2018

Module @babel/runtime/helpers/interopRequireDefault does not exist in the Haste module map #21310

Closed

matthargett approved these changes Sep 25, 2018

View reviewed changes

hramos mentioned this pull request Sep 25, 2018

Fix RNTester build failure in CI #21304

Closed

facebook-github-bot reviewed Sep 25, 2018

View reviewed changes

facebook-github-bot closed this in 458d56c Sep 26, 2018

facebook locked as resolved and limited conversation to collaborators Sep 26, 2018

react-native-bot added the Merged This PR has been merged. label Sep 26, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove global babelHelpers and regenerator #21283

Remove global babelHelpers and regenerator #21283

janicduplessis commented Sep 24, 2018 •

edited

Loading

This comment has been minimized.

rafeca commented Sep 24, 2018

rafeca commented Sep 24, 2018

facebook-github-bot left a comment

newyankeecodeshop Sep 24, 2018

janicduplessis Sep 24, 2018

janicduplessis commented Sep 24, 2018

janicduplessis commented Sep 24, 2018

janicduplessis commented Sep 24, 2018

kelset commented Sep 25, 2018

facebook-github-bot left a comment

janicduplessis commented Sep 25, 2018

kelset commented Sep 26, 2018

react-native-bot commented Sep 26, 2018

Remove global babelHelpers and regenerator #21283

Remove global babelHelpers and regenerator #21283

Conversation

janicduplessis commented Sep 24, 2018 • edited Loading

Test Plan:

Release Notes:

This comment has been minimized.

rafeca commented Sep 24, 2018

rafeca commented Sep 24, 2018

facebook-github-bot left a comment

Choose a reason for hiding this comment

newyankeecodeshop Sep 24, 2018

Choose a reason for hiding this comment

janicduplessis Sep 24, 2018

Choose a reason for hiding this comment

janicduplessis commented Sep 24, 2018

janicduplessis commented Sep 24, 2018

janicduplessis commented Sep 24, 2018

kelset commented Sep 25, 2018

facebook-github-bot left a comment

Choose a reason for hiding this comment

janicduplessis commented Sep 25, 2018

kelset commented Sep 26, 2018

react-native-bot commented Sep 26, 2018

janicduplessis commented Sep 24, 2018 •

edited

Loading