diff --git a/packages/metro-bundler/src/ModuleGraph/worker/__tests__/collect-dependencies-test.js b/packages/metro-bundler/src/ModuleGraph/worker/__tests__/collect-dependencies-test.js index 131f6f98..638159dc 100644 --- a/packages/metro-bundler/src/ModuleGraph/worker/__tests__/collect-dependencies-test.js +++ b/packages/metro-bundler/src/ModuleGraph/worker/__tests__/collect-dependencies-test.js @@ -18,6 +18,8 @@ const {codeFromAst, comparableCode} = require('../../test-helpers'); const {any} = expect; +const {InvalidRequireCallError} = collectDependencies; + describe('dependency collection from ASTs:', () => { it('collects dependency identifiers from the code', () => { const ast = astFromCode(` @@ -41,16 +43,20 @@ describe('dependency collection from ASTs:', () => { expect(collectDependencies(ast).dependencies).toEqual(['left-pad']); }); - it('ignores template literals with interpolations', () => { + it('throws on template literals with interpolations', () => { const ast = astFromCode('require(`left${"-"}pad`)'); - expect(collectDependencies(ast).dependencies).toEqual([]); + expect(() => collectDependencies(ast).dependencies).toThrowError( + InvalidRequireCallError, + ); }); - it('ignores tagged template literals', () => { + it('throws on tagged template literals', () => { const ast = astFromCode('require(tag`left-pad`)'); - expect(collectDependencies(ast).dependencies).toEqual([]); + expect(() => collectDependencies(ast).dependencies).toThrowError( + InvalidRequireCallError, + ); }); it('exposes a string as `dependencyMapName`', () => { @@ -66,7 +72,6 @@ describe('dependency collection from ASTs:', () => { it('replaces all required module ID strings with array lookups, keeps the ID as second argument', () => { const ast = astFromCode(` const a = require('b/lib/a'); - const b = require(123); exports.do = () => require("do"); if (!something) { require("setup/something"); @@ -78,7 +83,6 @@ describe('dependency collection from ASTs:', () => { expect(codeFromAst(ast)).toEqual( comparableCode(` const a = require(${dependencyMapName}[0], 'b/lib/a'); - const b = require(123); exports.do = () => require(${dependencyMapName}[1], "do"); if (!something) { require(${dependencyMapName}[2], "setup/something"); @@ -96,7 +100,6 @@ describe('Dependency collection from optimized ASTs:', () => { beforeEach(() => { ast = astFromCode(` const a = require(${dependencyMapName}[0], 'b/lib/a'); - const b = require(123); exports.do = () => require(${dependencyMapName}[1], "do"); if (!something) { require(${dependencyMapName}[2], "setup/something"); @@ -126,7 +129,6 @@ describe('Dependency collection from optimized ASTs:', () => { expect(codeFromAst(ast)).toEqual( comparableCode(` const a = require(${dependencyMapName}[0]); - const b = require(123); exports.do = () => require(${dependencyMapName}[1]); if (!something) { require(${dependencyMapName}[2]); diff --git a/packages/metro-bundler/src/ModuleGraph/worker/collect-dependencies.js b/packages/metro-bundler/src/ModuleGraph/worker/collect-dependencies.js index 03639fc8..571936c1 100644 --- a/packages/metro-bundler/src/ModuleGraph/worker/collect-dependencies.js +++ b/packages/metro-bundler/src/ModuleGraph/worker/collect-dependencies.js @@ -15,6 +15,7 @@ const nullthrows = require('fbjs/lib/nullthrows'); const {traverse, types} = require('babel-core'); +const prettyPrint = require('babel-generator').default; type AST = Object; @@ -27,13 +28,16 @@ class Replacement { this.nextIndex = 0; } - isRequireCall(callee, firstArg) { - return ( - callee.type === 'Identifier' && - callee.name === 'require' && - firstArg && - isLiteralString(firstArg) - ); + getRequireCallArg(node) { + const args = node.arguments; + if (args.length !== 1 || !isLiteralString(args[0])) { + throw new InvalidRequireCallError( + 'Calls to require() expect exactly 1 string literal argument, but ' + + 'this was found: ' + + prettyPrint(node).code, + ); + } + return args[0]; } getIndex(stringLiteralOrTemplateLiteral) { @@ -59,6 +63,14 @@ class Replacement { } } +function getInvalidProdRequireMessage(node) { + return ( + 'Post-transform calls to require() expect 2 arguments, the first ' + + 'of which has the shape `_dependencyMapName[123]`, but this was found: ' + + prettyPrint(node).code + ); +} + class ProdReplacement { replacement: Replacement; names: Array; @@ -68,15 +80,22 @@ class ProdReplacement { this.names = names; } - isRequireCall(callee, firstArg) { - return ( - callee.type === 'Identifier' && - callee.name === 'require' && - firstArg && - firstArg.type === 'MemberExpression' && - firstArg.property && - firstArg.property.type === 'NumericLiteral' - ); + getRequireCallArg(node) { + const args = node.arguments; + if (args.length !== 2) { + throw new InvalidRequireCallError(getInvalidProdRequireMessage(node)); + } + const arg = args[0]; + if ( + !( + arg.type === 'MemberExpression' && + arg.property && + arg.property.type === 'NumericLiteral' + ) + ) { + throw new InvalidRequireCallError(getInvalidProdRequireMessage(node)); + } + return args[0]; } getIndex(memberExpression) { @@ -111,6 +130,7 @@ function createMapLookup(dependencyMapIdentifier, propertyIdentifier) { } function collectDependencies(ast, replacement, dependencyMapIdentifier) { + const visited = new WeakSet(); const traversalState = {dependencyMapIdentifier}; traverse( ast, @@ -124,14 +144,18 @@ function collectDependencies(ast, replacement, dependencyMapIdentifier) { }, CallExpression(path, state) { const node = path.node; - const arg = node.arguments[0]; - if (replacement.isRequireCall(node.callee, arg)) { + if (visited.has(node)) { + return; + } + if (isRequireCall(node.callee)) { + const arg = replacement.getRequireCallArg(node); const index = replacement.getIndex(arg); node.arguments = replacement.makeArgs( types.numericLiteral(index), arg, state.dependencyMapIdentifier, ); + visited.add(node); } }, }, @@ -152,6 +176,16 @@ function isLiteralString(node) { ); } +function isRequireCall(callee) { + return callee.type === 'Identifier' && callee.name === 'require'; +} + +class InvalidRequireCallError extends Error { + constructor(message) { + super(message); + } +} + const xp = (module.exports = (ast: AST) => collectDependencies(ast, new Replacement())); @@ -165,3 +199,5 @@ xp.forOptimization = ( new ProdReplacement(names), dependencyMapName ? types.identifier(dependencyMapName) : undefined, ); + +xp.InvalidRequireCallError = InvalidRequireCallError;