Although we're now only visiting CallExpression nodes for the custom isRequire case, as opposed to all nodes, I'm not considering this a breaking change. There's been an implicit assumption that only CallExpression (and I guess NewExpression) nodes where being visited before this change. It's assumed that any node that isRequire returns true for has an arguments object. Returning true for any other type of node, would've thrown since no other types have an arguments property.
Benchmark results:
# before
$ for i in {1..5}; do node bench/detect.js; done
140
132
143
125
133
# after
$ for i in {1..5}; do node bench/detect.js; done
100
105
105
105
106
Fixes https://github.com/substack/node-detective/pull/53.
Although we're now only visiting
CallExpression
nodes for the customisRequire
case, as opposed to all nodes, I'm not considering this a breaking change. There's been an implicit assumption that onlyCallExpression
(and I guessNewExpression
) nodes where being visited before this change. It's assumed that any node thatisRequire
returns true for has anarguments
object. Returning true for any other type of node, would've thrown since no other types have anarguments
property.Benchmark results: