From 3a7077de4da997739f2617eac607a96e64c9a6c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9e=20Kooi?= Date: Mon, 12 Feb 2018 20:31:12 +0100 Subject: [PATCH 1/2] ignore require() calls inside already browserified bundles If a function expression declares a `require` parameter, its body is not analyzed for `require()` calls. This is mostly helpful for bundles that were already browserified, `detective` will ignore the calls to browser-pack's require runtime. --- index.js | 13 +++++++++++++ test/files/scope.js | 8 ++++++++ test/scope.js | 9 +++++++++ 3 files changed, 30 insertions(+) create mode 100644 test/files/scope.js create mode 100644 test/scope.js diff --git a/index.js b/index.js index 382d701a9..7313a348e 100644 --- a/index.js +++ b/index.js @@ -49,6 +49,7 @@ exports.find = function (src, opts) { function visit(node, st, c) { var hasRequire = wordRe.test(src.slice(node.start, node.end)); if (!hasRequire) return; + if (redefinesRequire(node)) return; walk.base[node.type](node, st, c); if (node.type !== 'CallExpression') return; if (isRequire(node)) { @@ -75,6 +76,18 @@ exports.find = function (src, opts) { Statement: visit, Expression: visit }); + + // Detect `require` redefinitions in function parameter lists, like + // in `[function(require,module,exports){` generated by browser-pack. + // This is a simple way to address the 99% case without doing full scope analysis + function redefinesRequire(node) { + if (node.type === 'FunctionExpression') { + return node.params.some(function (param) { + return param.type === 'Identifier' && param.name === word; + }); + } + return false; + } return modules; }; diff --git a/test/files/scope.js b/test/files/scope.js new file mode 100644 index 000000000..66ca0aed5 --- /dev/null +++ b/test/files/scope.js @@ -0,0 +1,8 @@ +(function(modules){ + modules[1](function(i){return modules[i]()}) +})({1:[function (require,module,exports) { + require('./y') +},{'./y':2}],2:function(require,module,exports){ + console.log("abc") +}}) +require('./z') diff --git a/test/scope.js b/test/scope.js new file mode 100644 index 000000000..e6b29bbb9 --- /dev/null +++ b/test/scope.js @@ -0,0 +1,9 @@ +var test = require('tap').test; +var detective = require('../'); +var fs = require('fs'); +var src = fs.readFileSync(__dirname + '/files/scope.js'); + +test('scope', function (t) { + t.plan(1); + t.deepEqual(detective(src), [ './z' ]); +}); From c9bfb246119b76119e39ce7a5e43ece033eca079 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9e=20Kooi?= Date: Fri, 13 Apr 2018 11:01:22 +0200 Subject: [PATCH 2/2] Be really strict about the kind of redefinition we expect --- index.js | 20 +++++++++++--------- test/files/scope.js | 6 +++++- test/scope.js | 2 +- 3 files changed, 17 insertions(+), 11 deletions(-) diff --git a/index.js b/index.js index 7313a348e..70cfc704a 100644 --- a/index.js +++ b/index.js @@ -49,7 +49,7 @@ exports.find = function (src, opts) { function visit(node, st, c) { var hasRequire = wordRe.test(src.slice(node.start, node.end)); if (!hasRequire) return; - if (redefinesRequire(node)) return; + if (isBundledDefinition(node)) return; walk.base[node.type](node, st, c); if (node.type !== 'CallExpression') return; if (isRequire(node)) { @@ -78,15 +78,17 @@ exports.find = function (src, opts) { }); // Detect `require` redefinitions in function parameter lists, like - // in `[function(require,module,exports){` generated by browser-pack. + // in `{0:[function(require,module,exports){` generated by browser-pack. // This is a simple way to address the 99% case without doing full scope analysis - function redefinesRequire(node) { - if (node.type === 'FunctionExpression') { - return node.params.some(function (param) { - return param.type === 'Identifier' && param.name === word; - }); - } - return false; + function isBundledDefinition(node) { + if (node.type !== 'ObjectExpression') return false; + if (node.properties.length < 1) return false; + var arr = node.properties[0].value; + if (arr.type !== 'ArrayExpression') return false; + if (arr.elements.length < 2) return false; + if (arr.elements[0].type !== 'FunctionExpression') return false; + var fn = arr.elements[0]; + return fn.params.length > 0 && fn.params[0].type === 'Identifier' && fn.params[0].name === word; } return modules; diff --git a/test/files/scope.js b/test/files/scope.js index 66ca0aed5..b94d787ac 100644 --- a/test/files/scope.js +++ b/test/files/scope.js @@ -1,8 +1,12 @@ (function(modules){ modules[1](function(i){return modules[i]()}) })({1:[function (require,module,exports) { - require('./y') + require('./y') // inside a bundle; should not be detected },{'./y':2}],2:function(require,module,exports){ console.log("abc") }}) + +(function (require) { + require('./x'); // not inside a bundle; should be detected +}(require)); // (because someone might do this) require('./z') diff --git a/test/scope.js b/test/scope.js index e6b29bbb9..3ffebca02 100644 --- a/test/scope.js +++ b/test/scope.js @@ -5,5 +5,5 @@ var src = fs.readFileSync(__dirname + '/files/scope.js'); test('scope', function (t) { t.plan(1); - t.deepEqual(detective(src), [ './z' ]); + t.deepEqual(detective(src), [ './x', './z' ]); });