Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Webpack-specific fix for issue #7 #33

Merged
merged 4 commits into from
Nov 1, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 68 additions & 22 deletions lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,25 @@ function optimizeJs (jsString, opts) {

walk(ast, {
enter: function (node, parent) {
// assuming this node is an argument to a function, return true if it itself
// is already padded with parentheses
// estree-walker does not automatically add a parent node pointer to nodes,
// which we need for some of our context checks below.
// normally I would just write "node.parentNode = parent" here, but that makes
// estree-walker think that parentNode is a child node of the node, which leads to
// infinite loops as it walks a circular tree. if we make parent a function, though,
// estree-walker does not follow the link.
node.parent = function () {
return parent
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agree this is a weird hack, but at least you marked it as such and added a lot of explanation :)

// assuming this node is an argument to a function or an element in an array,
// return true if it itself is already padded with parentheses
function isPaddedArgument (node) {
var idx = parent.arguments.indexOf(node)
var parentArray = node.parent().arguments ? node.parent().arguments : node.parent().elements
var idx = parentArray.indexOf(node)
if (idx === 0) { // first arg
if (prePreChar === '(' && preChar === '(' && postChar === ')') { // already padded
return true
}
} else if (idx === parent.arguments.length - 1) { // last arg
} else if (idx === parentArray.length - 1) { // last arg
if (preChar === '(' && postChar === ')' && postPostChar === ')') { // already padded
return true
}
Expand All @@ -31,30 +41,66 @@ function optimizeJs (jsString, opts) {
return false
}

function isCallExpression (node) {
return node && node.type === 'CallExpression'
}

function isArrayExpression (node) {
return node && node.type === 'ArrayExpression'
}

// returns true iff node is an argument to a function call expression.
function isArgumentToFunctionCall (node) {
return isCallExpression(node.parent()) &&
node.parent().arguments.length &&
node.parent().arguments.indexOf(node) !== -1
}

// returns true iff node is an element of an array literal which is in turn
// an argument to a function call expression.
function isElementOfArrayArgumentToFunctionCall (node) {
return isArrayExpression(node.parent()) &&
node.parent().elements.indexOf(node) !== -1 &&
isArgumentToFunctionCall(node.parent())
}

// returns true iff node is an IIFE.
function isIIFE (node) {
return isCallExpression(node.parent()) &&
node.parent().callee === node
}

// tries to divine if this function is a webpack module wrapper.
// returns true iff node is an element of an array literal which is in turn
// an argument to a function call expression, and that function call
// expression is an IIFE.
function isProbablyWebpackModule (node) {
return isElementOfArrayArgumentToFunctionCall(node) &&
node.parent() && // array literal
node.parent().parent() && // CallExpression
node.parent().parent().callee && // function that is being called
node.parent().parent().callee.type === 'FunctionExpression'
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

love the name of this function! 😂


if (node.type === 'FunctionExpression') {
var prePreChar = jsString.charAt(node.start - 2)
var preChar = jsString.charAt(node.start - 1)
var postChar = jsString.charAt(node.end)
var postPostChar = jsString.charAt(node.end + 1)

if (parent && parent.type === 'CallExpression') {
// this function is getting called itself or
// it is getting passed in to another call expression
// the else statement is strictly never hit, but I think the code is easier to read this way
/* istanbul ignore else */
if (parent.arguments && parent.arguments.indexOf(node) !== -1) {
// function passed in to another function. these are almost _always_ executed, e.g.
// UMD bundles, Browserify bundles, Node-style errbacks, Promise then()s and catch()s, etc.
if (!isPaddedArgument(node)) { // don't double-pad
magicString = magicString.insertLeft(node.start, '(')
.insertRight(node.end, ')')
}
} else if (parent.callee === node) {
// this function is getting immediately invoked, e.g. function(){}()
if (preChar !== '(') {
magicString.insertLeft(node.start, '(')
.insertRight(node.end, ')')
}
if (isArgumentToFunctionCall(node) || isProbablyWebpackModule(node)) {
// function passed in to another function, either as an argument, or as an element
// of an array argument. these are almost _always_ executed, e.g. webpack bundles,
// UMD bundles, Browserify bundles, Node-style errbacks, Promise then()s and catch()s, etc.
if (!isPaddedArgument(node)) { // don't double-pad
magicString = magicString.insertLeft(node.start, '(')
.insertRight(node.end, ')')
}
} else if (isIIFE(node)) {
// this function is getting immediately invoked, e.g. function(){}()
if (preChar !== '(') {
magicString.insertLeft(node.start, '(')
.insertRight(node.end, ')')
}
}
}
Expand Down
5 changes: 5 additions & 0 deletions test/cases/array-literal-iife-in-function-scope/input.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
function b() {
var a = [
!function() {return 1;}()
];
}
5 changes: 5 additions & 0 deletions test/cases/array-literal-iife-in-function-scope/output.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
function b() {
var a = [
!(function() {return 1;})()
];
}
3 changes: 3 additions & 0 deletions test/cases/array-literal-iife-in-global-scope/input.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
var a = [
!function() {return 1;}()
]
3 changes: 3 additions & 0 deletions test/cases/array-literal-iife-in-global-scope/output.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
var a = [
!(function() {return 1;})()
]
5 changes: 5 additions & 0 deletions test/cases/array-literal-in-function-params/input.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
foo([
function(o,r,t){
console.log("element 0");
}
]);
5 changes: 5 additions & 0 deletions test/cases/array-literal-in-function-params/output.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
foo([
function(o,r,t){
console.log("element 0");
}
]);
5 changes: 5 additions & 0 deletions test/cases/array-literal-in-function-scope/input.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
function b() {
var a = [
function() {return 1;}
];
}
5 changes: 5 additions & 0 deletions test/cases/array-literal-in-function-scope/output.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
function b() {
var a = [
function() {return 1;}
];
}
3 changes: 3 additions & 0 deletions test/cases/array-literal-in-global-scope/input.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
var a = [
function() {return 1;}
]
3 changes: 3 additions & 0 deletions test/cases/array-literal-in-global-scope/output.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
var a = [
function() {return 1;}
]
9 changes: 9 additions & 0 deletions test/cases/webpack-style-multi-element/input.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
!function(o){
return o[0]();
}([function(o,r,t){
console.log("webpack style element 0");
},function(o,r,t){
console.log("webpack style element 1");
},function(o,r,t){
console.log("webpack style element 2");
}]);
9 changes: 9 additions & 0 deletions test/cases/webpack-style-multi-element/output.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
!(function(o){
return o[0]();
})([(function(o,r,t){
console.log("webpack style element 0");
}),(function(o,r,t){
console.log("webpack style element 1");
}),(function(o,r,t){
console.log("webpack style element 2");
})]);
5 changes: 5 additions & 0 deletions test/cases/webpack-style-one-element/input.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
!function(o){
return o[0]();
}([function(o,r,t){
console.log("webpack style!");
}]);
5 changes: 5 additions & 0 deletions test/cases/webpack-style-one-element/output.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
!(function(o){
return o[0]();
})([(function(o,r,t){
console.log("webpack style!");
})]);