From 6ab9e7d09c50f41cff96bbf0a97f40bd8bc7cad4 Mon Sep 17 00:00:00 2001 From: Peter van der Zee Date: Tue, 10 Apr 2018 10:09:52 -0700 Subject: [PATCH] Change the name JSFileWrapping uses for `require` Reviewed By: jeanlauliac Differential Revision: D7502266 fbshipit-source-id: 23457eba7321fccb6d9a1f2d3d950e75cdc8a26a --- .../worker/__tests__/worker-test.js | 18 +- .../src/ModuleGraph/worker/JsFileWrapping.js | 11 +- .../worker/__tests__/JsFileWrapping-test.js | 157 +++++++++++++----- .../worker/__tests__/transform-module-test.js | 6 +- .../__snapshots__/basic_bundle-test.js.snap | 40 ++--- 5 files changed, 159 insertions(+), 73 deletions(-) diff --git a/packages/metro/src/JSTransformer/worker/__tests__/worker-test.js b/packages/metro/src/JSTransformer/worker/__tests__/worker-test.js index 6b345ece..b5cec744 100644 --- a/packages/metro/src/JSTransformer/worker/__tests__/worker-test.js +++ b/packages/metro/src/JSTransformer/worker/__tests__/worker-test.js @@ -67,7 +67,7 @@ describe('code transformation worker:', () => { expect(result.code).toBe( [ - '__d(function (global, _require, module, exports, _dependencyMap) {', + '__d(function (global, _$$_REQUIRE, module, exports, _dependencyMap) {', ' arbitrary(code);', '});', ].join('\n'), @@ -103,16 +103,16 @@ describe('code transformation worker:', () => { expect(BABEL_VERSION).toBe(7); expect(result.code).toBe( [ - '__d(function (global, _require, module, exports, _dependencyMap) {', + '__d(function (global, _$$_REQUIRE, module, exports, _dependencyMap) {', " 'use strict';", '', - ' var _c = babelHelpers.interopRequireDefault(_require(_dependencyMap[0], "./c"));', + ' var _c = babelHelpers.interopRequireDefault(_$$_REQUIRE(_dependencyMap[0], "./c"));', '', - ' _require(_dependencyMap[1], "./a");', + ' _$$_REQUIRE(_dependencyMap[1], "./a");', '', ' arbitrary(code);', '', - ' var b = _require(_dependencyMap[2], "b");', + ' var b = _$$_REQUIRE(_dependencyMap[2], "b");', '});', ].join('\n'), ); @@ -145,16 +145,16 @@ describe('code transformation worker:', () => { expect(BABEL_VERSION).toBe(6); expect(result.code).toBe( [ - '__d(function (global, _require, module, exports, _dependencyMap) {', - ' var _c = _require(_dependencyMap[0], "./c");', + '__d(function (global, _$$_REQUIRE, module, exports, _dependencyMap) {', + ' var _c = _$$_REQUIRE(_dependencyMap[0], "./c");', '', ' var _c2 = babelHelpers.interopRequireDefault(_c);', '', - ' _require(_dependencyMap[1], "./a");', + ' _$$_REQUIRE(_dependencyMap[1], "./a");', '', ' arbitrary(code);', '', - ' var b = _require(_dependencyMap[2], "b");', + ' var b = _$$_REQUIRE(_dependencyMap[2], "b");', '});', ].join('\n'), ); diff --git a/packages/metro/src/ModuleGraph/worker/JsFileWrapping.js b/packages/metro/src/ModuleGraph/worker/JsFileWrapping.js index cdb6a542..749c916f 100644 --- a/packages/metro/src/ModuleGraph/worker/JsFileWrapping.js +++ b/packages/metro/src/ModuleGraph/worker/JsFileWrapping.js @@ -15,6 +15,7 @@ const {babelTypes, babelTraverse: traverse} = require('../../babel-bridge'); const MODULE_FACTORY_PARAMETERS = ['global', 'require', 'module', 'exports']; const POLYFILL_FACTORY_PARAMETERS = ['global']; +const WRAP_NAME = '$$_REQUIRE'; // note: babel will prefix this with _ function wrapModule( fileAst: Object, @@ -66,21 +67,23 @@ function makeIdentifier(name: string): Object { } function renameRequires(ast: Object) { - let requireName = 'require'; + let newRequireName = WRAP_NAME; traverse(ast, { Program(path) { const body = path.get('body.0.expression.arguments.0.body'); - requireName = body.scope.generateUid('_require'); - body.scope.rename('require', requireName); + newRequireName = body.scope.generateUid(WRAP_NAME); + body.scope.rename('require', newRequireName); }, }); - return requireName; + return newRequireName; } module.exports = { + WRAP_NAME, + wrapJson, wrapModule, wrapPolyfill, diff --git a/packages/metro/src/ModuleGraph/worker/__tests__/JsFileWrapping-test.js b/packages/metro/src/ModuleGraph/worker/__tests__/JsFileWrapping-test.js index ae8fb107..818a97a5 100644 --- a/packages/metro/src/ModuleGraph/worker/__tests__/JsFileWrapping-test.js +++ b/packages/metro/src/ModuleGraph/worker/__tests__/JsFileWrapping-test.js @@ -16,6 +16,12 @@ const JsFileWrapping = require('../JsFileWrapping'); const {babylon} = require('../../../babel-bridge'); const {codeFromAst, comparableCode} = require('../../test-helpers'); +const {WRAP_NAME} = JsFileWrapping; +// Note; it's not important HOW Babel changes the name. Only THAT it does. +// At the time of writing, it will prefix an underscore for our first rename +const BABEL_RENAMED = '_' + WRAP_NAME; +const BABEL_RENAMED2 = '_' + WRAP_NAME + '2'; + it('wraps a module correctly', () => { const dependencyMapName = '_dependencyMapName'; @@ -33,53 +39,130 @@ it('wraps a module correctly', () => { dependencyMapName, ); - expect(requireName).toBe('_require'); + expect(requireName).toBe(BABEL_RENAMED); expect(codeFromAst(ast)).toEqual( comparableCode(` - __d(function (global, _require, module, exports, _dependencyMapName) { - const dynamicRequire = _require; - const a = _require('b/lib/a'); - exports.do = () => _require("do"); + __d(function (global, ${BABEL_RENAMED}, module, exports, _dependencyMapName) { + const dynamicRequire = ${BABEL_RENAMED}; + const a = ${BABEL_RENAMED}('b/lib/a'); + exports.do = () => ${BABEL_RENAMED}("do"); if (!something) { - _require("setup/something"); + ${BABEL_RENAMED}("setup/something"); } - _require.blah('do'); + ${BABEL_RENAMED}.blah('do'); });`), ); }); -it('replaces the require variable by a unique one', () => { - const dependencyMapName = '_dependencyMapName'; +describe('safe renaming of require', () => { + ['let', 'const', 'var'].forEach(declKeyword => { + describe('decl type = ' + declKeyword, () => { + it(`original name will always be renamed so local decl should be fine`, () => { + const dependencyMapName = '_dependencyMapName'; - const originalAst = astFromCode(` - const dynamicRequire = require; - const a = require('b/lib/a'); - let _require = 'foo'; - exports.do = () => require("do"); - if (!something) { - require("setup/something"); - } - require.blah('do'); - `); - const {ast, requireName} = JsFileWrapping.wrapModule( - originalAst, - dependencyMapName, - ); + const originalAst = astFromCode(` + const dynamicRequire = require; + const a = require('b/lib/a'); + ${declKeyword} ${WRAP_NAME} = 'foo'; + exports.do = () => require("do"); + if (!something) { + require("setup/something"); + } + require.blah('do'); + `); + const {ast, requireName} = JsFileWrapping.wrapModule( + originalAst, + dependencyMapName, + ); - expect(requireName).toBe('_require2'); - expect(codeFromAst(ast)).toEqual( - comparableCode(` - __d(function (global, _require2, module, exports, _dependencyMapName) { - const dynamicRequire = _require2; - const a = _require2('b/lib/a'); - let _require = 'foo'; - exports.do = () => _require2("do"); - if (!something) { - _require2("setup/something"); - } - _require2.blah('do'); - });`), - ); + expect(requireName).toBe(BABEL_RENAMED); + expect(codeFromAst(ast)).toEqual( + comparableCode(` + __d(function (global, ${BABEL_RENAMED}, module, exports, _dependencyMapName) { + const dynamicRequire = ${BABEL_RENAMED}; + const a = ${BABEL_RENAMED}('b/lib/a'); + ${declKeyword} ${WRAP_NAME} = 'foo'; + exports.do = () => ${BABEL_RENAMED}("do"); + if (!something) { + ${BABEL_RENAMED}("setup/something"); + } + ${BABEL_RENAMED}.blah('do'); + });`), + ); + }); + + it(`when the scope has the new name defined too`, () => { + const dependencyMapName = '_dependencyMapName'; + + const originalAst = astFromCode(` + const dynamicRequire = require; + const a = require('b/lib/a'); + ${declKeyword} ${BABEL_RENAMED} = 'foo'; + exports.do = () => require("do"); + if (!something) { + require("setup/something"); + } + require.blah('do'); + `); + const {ast, requireName} = JsFileWrapping.wrapModule( + originalAst, + dependencyMapName, + ); + + expect(requireName).toBe(BABEL_RENAMED2); + expect(codeFromAst(ast)).toEqual( + comparableCode(` + __d(function (global, ${BABEL_RENAMED2}, module, exports, _dependencyMapName) { + const dynamicRequire = ${BABEL_RENAMED2}; + const a = ${BABEL_RENAMED2}('b/lib/a'); + ${declKeyword} ${BABEL_RENAMED} = 'foo'; + exports.do = () => ${BABEL_RENAMED2}("do"); + if (!something) { + ${BABEL_RENAMED2}("setup/something"); + } + ${BABEL_RENAMED2}.blah('do'); + });`), + ); + }); + + it(`when an inner scope already has the new name defined too`, () => { + const dependencyMapName = '_dependencyMapName'; + + // Note; it's not important HOW Babel changes the name. Only THAT it does. + const BABEL_RENAMED = '_' + WRAP_NAME; + + const originalAst = astFromCode(` + const dynamicRequire = require; + const a = require('b/lib/a'); + if (a) { + (function () { + ${declKeyword} ${BABEL_RENAMED} = require('dingus'); + a(${BABEL_RENAMED}(dynamicRequire)); + }) + } + `); + const {ast, requireName} = JsFileWrapping.wrapModule( + originalAst, + dependencyMapName, + ); + + expect(requireName).toBe(BABEL_RENAMED2); + expect(codeFromAst(ast)).toEqual( + comparableCode(` + __d(function (global, ${BABEL_RENAMED2}, module, exports, _dependencyMapName) { + const dynamicRequire = ${BABEL_RENAMED2}; + const a = ${BABEL_RENAMED2}('b/lib/a'); + if (a) { + (function () { + ${declKeyword} ${BABEL_RENAMED} = ${BABEL_RENAMED2}('dingus'); + a(${BABEL_RENAMED}(dynamicRequire)); + }); + } + });`), + ); + }); + }); + }); }); it('wraps a polyfill correctly', () => { diff --git a/packages/metro/src/ModuleGraph/worker/__tests__/transform-module-test.js b/packages/metro/src/ModuleGraph/worker/__tests__/transform-module-test.js index 46912f18..9a49bcf2 100644 --- a/packages/metro/src/ModuleGraph/worker/__tests__/transform-module-test.js +++ b/packages/metro/src/ModuleGraph/worker/__tests__/transform-module-test.js @@ -135,7 +135,7 @@ describe('transforming JS modules:', () => { const {code, dependencyMapName} = result.details.transformed.default; invariant(dependencyMapName != null, 'dependencyMapName cannot be null'); expect(code.replace(/\s+/g, '')).toEqual( - `__d(function(global,_require,module,exports,${dependencyMapName}){${transformedCode}});`, + `__d(function(global,_$$_REQUIRE,module,exports,${dependencyMapName}){${transformedCode}});`, ); }); @@ -192,12 +192,12 @@ describe('transforming JS modules:', () => { invariant(result.type === 'code', 'result must be code'); const {dev, prod} = result.details.transformed; expect(dev.code.replace(/\s+/g, '')).toEqual( - `__d(function(global,_require,module,exports,${nullthrows( + `__d(function(global,_$$_REQUIRE,module,exports,${nullthrows( dev.dependencyMapName, )}){arbitrary(code);});`, ); expect(prod.code.replace(/\s+/g, '')).toEqual( - `__d(function(global,_require,module,exports,${nullthrows( + `__d(function(global,_$$_REQUIRE,module,exports,${nullthrows( prod.dependencyMapName, )}){arbitrary(code);});`, ); diff --git a/packages/metro/src/integration_tests/__tests__/__snapshots__/basic_bundle-test.js.snap b/packages/metro/src/integration_tests/__tests__/__snapshots__/basic_bundle-test.js.snap index d34c2ca4..73288321 100644 --- a/packages/metro/src/integration_tests/__tests__/__snapshots__/basic_bundle-test.js.snap +++ b/packages/metro/src/integration_tests/__tests__/__snapshots__/basic_bundle-test.js.snap @@ -138,40 +138,40 @@ exports[`basic_bundle bundles package with polyfills 1`] = ` String.prototype.repeat = function () {}; } })(this); -__d(function (global, _require, module, exports, _dependencyMap) { +__d(function (global, _$$_REQUIRE, module, exports, _dependencyMap) { 'use strict'; - var Bar = _require(_dependencyMap[0]); + var Bar = _$$_REQUIRE(_dependencyMap[0]); - var Foo = _require(_dependencyMap[1]); + var Foo = _$$_REQUIRE(_dependencyMap[1]); module.exports = { Foo: Foo, Bar: Bar }; },0,[1,2]); -__d(function (global, _require, module, exports, _dependencyMap) { +__d(function (global, _$$_REQUIRE, module, exports, _dependencyMap) { 'use strict'; - var Foo = _require(_dependencyMap[0]); + var Foo = _$$_REQUIRE(_dependencyMap[0]); module.exports = { type: 'bar', foo: Foo.type }; },1,[2]); -__d(function (global, _require, module, exports, _dependencyMap) { +__d(function (global, _$$_REQUIRE, module, exports, _dependencyMap) { 'use strict'; - var asset = _require(_dependencyMap[0]); + var asset = _$$_REQUIRE(_dependencyMap[0]); module.exports = { type: 'foo', asset: asset }; },2,[3]); -__d(function (global, _require, module, exports, _dependencyMap) { - module.exports = _require(_dependencyMap[0]).registerAsset({ +__d(function (global, _$$_REQUIRE, module, exports, _dependencyMap) { + module.exports = _$$_REQUIRE(_dependencyMap[0]).registerAsset({ \\"__packager_asset\\": true, \\"httpServerLocation\\": \\"/assets\\", \\"width\\": 8, @@ -182,7 +182,7 @@ __d(function (global, _require, module, exports, _dependencyMap) { \\"type\\": \\"png\\" }); },3,[4]); -__d(function (global, _require, module, exports, _dependencyMap) { +__d(function (global, _$$_REQUIRE, module, exports, _dependencyMap) { 'use strict'; },4,[]); require(0);" @@ -312,40 +312,40 @@ exports[`basic_bundle bundles package without polyfills 1`] = ` return Error('Requiring module \\"' + displayName + '\\", which threw an exception: ' + error); } })(this); -__d(function (global, _require, module, exports, _dependencyMap) { +__d(function (global, _$$_REQUIRE, module, exports, _dependencyMap) { 'use strict'; - var Bar = _require(_dependencyMap[0]); + var Bar = _$$_REQUIRE(_dependencyMap[0]); - var Foo = _require(_dependencyMap[1]); + var Foo = _$$_REQUIRE(_dependencyMap[1]); module.exports = { Foo: Foo, Bar: Bar }; },0,[1,2]); -__d(function (global, _require, module, exports, _dependencyMap) { +__d(function (global, _$$_REQUIRE, module, exports, _dependencyMap) { 'use strict'; - var Foo = _require(_dependencyMap[0]); + var Foo = _$$_REQUIRE(_dependencyMap[0]); module.exports = { type: 'bar', foo: Foo.type }; },1,[2]); -__d(function (global, _require, module, exports, _dependencyMap) { +__d(function (global, _$$_REQUIRE, module, exports, _dependencyMap) { 'use strict'; - var asset = _require(_dependencyMap[0]); + var asset = _$$_REQUIRE(_dependencyMap[0]); module.exports = { type: 'foo', asset: asset }; },2,[3]); -__d(function (global, _require, module, exports, _dependencyMap) { - module.exports = _require(_dependencyMap[0]).registerAsset({ +__d(function (global, _$$_REQUIRE, module, exports, _dependencyMap) { + module.exports = _$$_REQUIRE(_dependencyMap[0]).registerAsset({ \\"__packager_asset\\": true, \\"httpServerLocation\\": \\"/assets\\", \\"width\\": 8, @@ -356,7 +356,7 @@ __d(function (global, _require, module, exports, _dependencyMap) { \\"type\\": \\"png\\" }); },3,[4]); -__d(function (global, _require, module, exports, _dependencyMap) { +__d(function (global, _$$_REQUIRE, module, exports, _dependencyMap) { 'use strict'; },4,[]); require(0);"