From 8932a9caa8f9ffe82160f2da25342a0bde3bbb26 Mon Sep 17 00:00:00 2001 From: Janic Duplessis Date: Fri, 21 Sep 2018 16:05:10 -0700 Subject: [PATCH] Use babel runtime instead of relying on global babelHelpers and regenerator (#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 https://github.com/facebook/react-native/issues/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: https://github.com/facebook/metro/pull/198 Reviewed By: mjesun Differential Revision: D8833903 Pulled By: rafeca fbshipit-source-id: 7081f769f288ab358ba89ae8ee72a513bb12e225 --- .../package.json | 1 + .../src/configs/main.js | 12 ++++ .../__snapshots__/worker-test.js.snap | 59 +++++++++++++------ .../worker/__tests__/worker-test.js | 45 +++++++++++++- .../lib/polyfills/__tests__/require-test.js | 11 +--- packages/metro/src/reactNativeTransformer.js | 4 +- yarn.lock | 15 +++++ 7 files changed, 116 insertions(+), 31 deletions(-) diff --git a/packages/metro-react-native-babel-preset/package.json b/packages/metro-react-native-babel-preset/package.json index f1c78fa30e..567a860a7d 100644 --- a/packages/metro-react-native-babel-preset/package.json +++ b/packages/metro-react-native-babel-preset/package.json @@ -43,6 +43,7 @@ "@babel/plugin-transform-react-jsx": "^7.0.0", "@babel/plugin-transform-react-jsx-source": "^7.0.0", "@babel/plugin-transform-regenerator": "^7.0.0", + "@babel/plugin-transform-runtime": "^7.0.0", "@babel/plugin-transform-shorthand-properties": "^7.0.0", "@babel/plugin-transform-spread": "^7.0.0", "@babel/plugin-transform-sticky-regex": "^7.0.0", diff --git a/packages/metro-react-native-babel-preset/src/configs/main.js b/packages/metro-react-native-babel-preset/src/configs/main.js index 51a41127a2..4264010811 100644 --- a/packages/metro-react-native-babel-preset/src/configs/main.js +++ b/packages/metro-react-native-babel-preset/src/configs/main.js @@ -80,6 +80,14 @@ const reactDisplayName = [ const reactJsxSource = [require('@babel/plugin-transform-react-jsx-source')]; const symbolMember = [require('../transforms/transform-symbol-member')]; +const babelRuntime = [ + require('@babel/plugin-transform-runtime'), + { + helpers: true, + regenerator: true, + }, +]; + const getPreset = (src, options) => { const isNull = src == null; const hasClass = isNull || src.indexOf('class') !== -1; @@ -135,6 +143,10 @@ const getPreset = (src, options) => { extraPlugins.push(reactJsxSource); } + if (!options || !options.disableBabelRuntime) { + extraPlugins.push(babelRuntime); + } + return { comments: false, compact: true, diff --git a/packages/metro/src/JSTransformer/worker/__tests__/__snapshots__/worker-test.js.snap b/packages/metro/src/JSTransformer/worker/__tests__/__snapshots__/worker-test.js.snap index 3115122e6b..e1bc9c4e66 100644 --- a/packages/metro/src/JSTransformer/worker/__tests__/__snapshots__/worker-test.js.snap +++ b/packages/metro/src/JSTransformer/worker/__tests__/__snapshots__/worker-test.js.snap @@ -11,110 +11,110 @@ Array [ 0, ], Array [ - 4, + 6, 0, 5, 0, ], Array [ - 6, + 8, 0, 2, 0, "require", ], Array [ - 6, + 8, 2, 2, 0, "require", ], Array [ - 6, + 8, 13, 2, 7, ], Array [ - 6, + 8, 39, 2, 0, ], Array [ - 8, + 10, 0, 3, 0, "arbitrary", ], Array [ - 8, + 10, 2, 3, 0, "arbitrary", ], Array [ - 8, + 10, 11, 3, 9, ], Array [ - 8, + 10, 12, 3, 10, "code", ], Array [ - 8, + 10, 16, 3, 9, ], Array [ - 8, + 10, 17, 3, 0, ], Array [ - 10, + 12, 0, 4, 0, ], Array [ - 10, + 12, 6, 4, 6, "b", ], Array [ - 10, + 12, 7, 4, 7, ], Array [ - 10, + 12, 10, 4, 10, "require", ], Array [ - 10, + 12, 21, 4, 17, ], Array [ - 10, + 12, 45, 4, 0, @@ -210,6 +210,31 @@ Array [ ] `; +exports[`code transformation worker: transforms an es module with regenerator 1`] = ` +"__d(function (global, _$$_REQUIRE, _$$_IMPORT_DEFAULT, _$$_IMPORT_ALL, module, exports, _dependencyMap) { + var _interopRequireDefault = _$$_REQUIRE(_dependencyMap[0], \\"@babel/runtime/helpers/interopRequireDefault\\"); + + Object.defineProperty(exports, \\"__esModule\\", { + value: true + }); + exports.test = test; + + var _regenerator = _interopRequireDefault(_$$_REQUIRE(_dependencyMap[1], \\"@babel/runtime/regenerator\\")); + + function test() { + return _regenerator.default.async(function test$(_context) { + while (1) { + switch (_context.prev = _context.next) { + case 0: + case \\"end\\": + return _context.stop(); + } + } + }, null, this); + } +});" +`; + exports[`code transformation worker: transforms import/export syntax when experimental flag is on 1`] = ` Array [ Array [ diff --git a/packages/metro/src/JSTransformer/worker/__tests__/worker-test.js b/packages/metro/src/JSTransformer/worker/__tests__/worker-test.js index 3fe5c01293..e57dcb35d2 100644 --- a/packages/metro/src/JSTransformer/worker/__tests__/worker-test.js +++ b/packages/metro/src/JSTransformer/worker/__tests__/worker-test.js @@ -142,24 +142,63 @@ describe('code transformation worker:', () => { '__d(function (global, _$$_REQUIRE, _$$_IMPORT_DEFAULT, _$$_IMPORT_ALL, module, exports, _dependencyMap) {', " 'use strict';", '', - ' var _c = babelHelpers.interopRequireDefault(_$$_REQUIRE(_dependencyMap[0], "./c"));', + ' var _interopRequireDefault = _$$_REQUIRE(_dependencyMap[0], "@babel/runtime/helpers/interopRequireDefault");', '', - ' _$$_REQUIRE(_dependencyMap[1], "./a");', + ' var _c = _interopRequireDefault(_$$_REQUIRE(_dependencyMap[1], "./c"));', + '', + ' _$$_REQUIRE(_dependencyMap[2], "./a");', '', ' arbitrary(code);', '', - ' var b = _$$_REQUIRE(_dependencyMap[2], "b");', + ' var b = _$$_REQUIRE(_dependencyMap[3], "b");', '});', ].join('\n'), ); expect(result.output[0].data.map).toMatchSnapshot(); expect(result.dependencies).toEqual([ + { + data: {isAsync: false}, + name: '@babel/runtime/helpers/interopRequireDefault', + }, {data: {isAsync: false}, name: './c'}, {data: {isAsync: false}, name: './a'}, {data: {isAsync: false}, name: 'b'}, ]); }); + it('transforms an es module with regenerator', async () => { + const result = await transform( + '/root/local/file.js', + 'local/file.js', + 'export async function test() {}', + { + assetExts: [], + assetPlugins: [], + assetRegistryPath: '', + asyncRequireModulePath: 'asyncRequire', + isScript: false, + minifierPath: 'minifyModulePath', + babelTransformerPath, + transformOptions: {dev: true}, + dynamicDepsInPackages: 'reject', + }, + ); + + expect(result.output[0].type).toBe('js/module'); + expect(result.output[0].data.code).toMatchSnapshot(); + expect(result.output[0].data.map).toHaveLength(13); + expect(result.dependencies).toEqual([ + { + data: {isAsync: false}, + name: '@babel/runtime/helpers/interopRequireDefault', + }, + { + data: {isAsync: false}, + name: '@babel/runtime/regenerator', + }, + ]); + }); + it('transforms import/export syntax when experimental flag is on', async () => { const contents = ['import c from "./c";'].join('\n'); diff --git a/packages/metro/src/lib/polyfills/__tests__/require-test.js b/packages/metro/src/lib/polyfills/__tests__/require-test.js index d095079b75..7d8e29b47f 100644 --- a/packages/metro/src/lib/polyfills/__tests__/require-test.js +++ b/packages/metro/src/lib/polyfills/__tests__/require-test.js @@ -14,10 +14,6 @@ const fs = require('fs'); const {transformSync} = require('@babel/core'); -// Include the external-helpers plugin to be able to detect if they're -// needed when transforming the requirejs implementation. -const PLUGINS = ['@babel/plugin-external-helpers']; - function createModule( moduleSystem, moduleId, @@ -34,7 +30,6 @@ describe('require', () => { return transformSync(rawCode, { ast: false, babelrc: false, - plugins: PLUGINS.map(require), presets: [require.resolve('metro-react-native-babel-preset')], retainLines: true, sourceMaps: 'inline', @@ -56,9 +51,9 @@ describe('require', () => { }); it('does not need any babel helper logic', () => { - // Super-simple check to validate that no babel helpers are used. - // This check will need to be updated if https://fburl.com/6z0y2kf8 changes. - expect(moduleSystemCode.includes('babelHelpers')).toBe(false); + // The react native preset uses @babel/transform-runtime so helpers will be + // imported from @babel/runtime. + expect(moduleSystemCode.includes('@babel/runtime')).toBe(false); }); it('works with plain bundles', () => { diff --git a/packages/metro/src/reactNativeTransformer.js b/packages/metro/src/reactNativeTransformer.js index 2f5e37c296..b202958e37 100644 --- a/packages/metro/src/reactNativeTransformer.js +++ b/packages/metro/src/reactNativeTransformer.js @@ -12,7 +12,6 @@ 'use strict'; const crypto = require('crypto'); -const externalHelpersPlugin = require('@babel/plugin-external-helpers'); const fs = require('fs'); const inlineRequiresPlugin = require('babel-preset-fbjs/plugins/inline-requires'); const makeHMRConfig = require('metro-react-native-babel-preset/src/configs/hmr'); @@ -25,7 +24,6 @@ import type {Plugins as BabelPlugins} from 'babel-core'; const cacheKeyParts = [ fs.readFileSync(__filename), - require('@babel/plugin-external-helpers/package.json').version, require('babel-preset-fbjs/package.json').version, ]; @@ -118,7 +116,7 @@ function buildBabelConfig(filename, options, plugins?: BabelPlugins = []) { let config = Object.assign({}, babelRC, extraConfig); // Add extra plugins - const extraPlugins = [externalHelpersPlugin]; + const extraPlugins = []; if (options.inlineRequires) { extraPlugins.push(inlineRequiresPlugin); diff --git a/yarn.lock b/yarn.lock index 031fd80bbc..4045bd5910 100644 --- a/yarn.lock +++ b/yarn.lock @@ -632,6 +632,15 @@ dependencies: regenerator-transform "^0.13.3" +"@babel/plugin-transform-runtime@^7.0.0": + version "7.0.0" + resolved "https://registry.yarnpkg.com/@babel/plugin-transform-runtime/-/plugin-transform-runtime-7.0.0.tgz#0f1443c07bac16dba8efa939e0c61d6922740062" + integrity sha512-yECRVxRu25Nsf6IY5v5XrXhcW9ZHomUQiq30VO8H7r3JYPcBJDTcxZmT+6v1O3QKKrDp1Wp40LinGbcd+jlp9A== + dependencies: + "@babel/helper-module-imports" "^7.0.0" + "@babel/helper-plugin-utils" "^7.0.0" + resolve "^1.8.1" + "@babel/plugin-transform-shorthand-properties@^7.0.0": version "7.0.0" resolved "https://registry.yarnpkg.com/@babel/plugin-transform-shorthand-properties/-/plugin-transform-shorthand-properties-7.0.0.tgz#85f8af592dcc07647541a0350e8c95c7bf419d15" @@ -6753,6 +6762,12 @@ resolve@^1.3.2, resolve@^1.5.0: dependencies: path-parse "^1.0.5" +resolve@^1.8.1: + version "1.8.1" + resolved "https://registry.yarnpkg.com/resolve/-/resolve-1.8.1.tgz#82f1ec19a423ac1fbd080b0bab06ba36e84a7a26" + dependencies: + path-parse "^1.0.5" + restore-cursor@^2.0.0: version "2.0.0" resolved "https://registry.yarnpkg.com/restore-cursor/-/restore-cursor-2.0.0.tgz#9f7ee287f82fd326d4fd162923d62129eee0dfaf"