Skip to content

Commit c9ff97f

Browse files
ooflorentooflorent
committed
Eliminate unreachable branches
Webpack does not transpile unreachable branches leaving `import` expressions in the code. This PR modifies `ConstPlugin` to remove the unreachable branch. Before: ```js if (true) { 1 } else { import("a") } if (false) { import("a") } else { 1 } true ? 1 : import("a"); false ? import("a") : 1; ``` After: ```js if (true) { 1 } else {} if (false) {} else { 1 } true ? 1 : null; false ? null : 1; ```
1 parent d60a9f5 commit c9ff97f

File tree

6 files changed

+235
-3
lines changed

6 files changed

+235
-3
lines changed

‎lib/ConstPlugin.js

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,88 @@ const getQuery = (request) => {
1212
return request.indexOf("?") < 0 ? "" : request.substr(i);
1313
};
1414

15+
const collectDeclaration = (declarations, pattern) => {
16+
const stack = [pattern];
17+
while(stack.length > 0) {
18+
const node = stack.pop();
19+
switch(node.type) {
20+
case "Identifier":
21+
declarations.add(node.name);
22+
break;
23+
case "ArrayPattern":
24+
for(const element of node.elements)
25+
if(element) stack.push(element);
26+
break;
27+
case "AssignmentPattern":
28+
stack.push(node.left);
29+
break;
30+
case "ObjectPattern":
31+
for(const property of node.properties)
32+
stack.push(property.value);
33+
break;
34+
case "RestElement":
35+
stack.push(node.argument);
36+
break;
37+
}
38+
}
39+
};
40+
41+
const getHoistedDeclarations = (branch, includeFunctionDeclarations) => {
42+
const declarations = new Set();
43+
const stack = [branch];
44+
while(stack.length > 0) {
45+
const node = stack.pop();
46+
// Some node could be `null` or `undefined`.
47+
if(!node)
48+
continue;
49+
switch(node.type) {
50+
// Walk through control statements to look for hoisted declarations.
51+
// Some branches are skipped since they do not allow declarations.
52+
case "BlockStatement":
53+
for(const stmt of node.body)
54+
stack.push(stmt);
55+
break;
56+
case "IfStatement":
57+
stack.push(node.consequent);
58+
stack.push(node.alternate);
59+
break;
60+
case "ForStatement":
61+
stack.push(node.init);
62+
stack.push(node.body);
63+
break;
64+
case "ForInStatement":
65+
case "ForOfStatement":
66+
stack.push(node.left);
67+
stack.push(node.body);
68+
break;
69+
case "DoWhileStatement":
70+
case "WhileStatement":
71+
case "LabeledStatement":
72+
stack.push(node.body);
73+
break;
74+
case "SwitchStatement":
75+
for(const cs of node.cases)
76+
for(const consequent of cs.consequent)
77+
stack.push(consequent);
78+
break;
79+
case "TryStatement":
80+
stack.push(node.block);
81+
stack.push(node.finalizer);
82+
break;
83+
case "FunctionDeclaration":
84+
if(includeFunctionDeclarations)
85+
collectDeclaration(declarations, node.id);
86+
break;
87+
case "VariableDeclaration":
88+
if(node.kind === "var")
89+
for(const decl of node.declarations)
90+
collectDeclaration(declarations, decl.id);
91+
break;
92+
}
93+
}
94+
return Array.from(declarations);
95+
};
96+
1597
class ConstPlugin {
1698
apply(compiler) {
1799
compiler.hooks.compilation.tap("ConstPlugin", (compilation, {
@@ -30,6 +112,57 @@ class ConstPlugin {
30112
dep.loc = statement.loc;
31113
parser.state.current.addDependency(dep);
32114
}
115+
const branchToRemove = bool ? statement.alternate : statement.consequent;
116+
if(branchToRemove) {
117+
// Before removing the dead branch, the hoisted declarations
118+
// must be collected.
119+
//
120+
// Given the following code:
121+
//
122+
// if (true) f() else g()
123+
// if (false) {
124+
// function f() {}
125+
// const g = function g() {}
126+
// if (someTest) {
127+
// let a = 1
128+
// var x, {y, z} = obj
129+
// }
130+
// } else {
131+
// …
132+
// }
133+
//
134+
// the generated code is:
135+
//
136+
// if (true) f() else {}
137+
// if (false) {
138+
// var f, x, y, z; (in loose mode)
139+
// var x, y, z; (in strict mode)
140+
// } else {
141+
// …
142+
// }
143+
//
144+
// NOTE: When code runs in strict mode, `var` declarations
145+
// are hoisted but `function` declarations don't.
146+
//
147+
let declarations;
148+
if(parser.scope.isStrict) {
149+
// If the code runs in strict mode, variable declarations
150+
// using `var` must be hoisted.
151+
declarations = getHoistedDeclarations(branchToRemove, false);
152+
} else {
153+
// Otherwise, collect all hoisted declaration.
154+
declarations = getHoistedDeclarations(branchToRemove, true);
155+
}
156+
let replacement;
157+
if(declarations.length > 0) {
158+
replacement = `{ var ${declarations.join(", ")}; }`;
159+
} else {
160+
replacement = "{}";
161+
}
162+
const dep = new ConstDependency(replacement, branchToRemove.range);
163+
dep.loc = branchToRemove.loc;
164+
parser.state.current.addDependency(dep);
165+
}
33166
return bool;
34167
}
35168
});
@@ -42,6 +175,21 @@ class ConstPlugin {
42175
dep.loc = expression.loc;
43176
parser.state.current.addDependency(dep);
44177
}
178+
// Expressions do not hoist.
179+
// It is safe to remove the dead branch.
180+
//
181+
// Given the following code:
182+
//
183+
// false ? someExpression() : otherExpression();
184+
//
185+
// the generated code is:
186+
//
187+
// false ? undefined : otherExpression();
188+
//
189+
const branchToRemove = bool ? expression.alternate : expression.consequent;
190+
const dep = new ConstDependency("undefined", branchToRemove.range);
191+
dep.loc = branchToRemove.loc;
192+
parser.state.current.addDependency(dep);
45193
return bool;
46194
}
47195
});

‎lib/Parser.js

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -948,6 +948,7 @@ class Parser extends Tapable {
948948
this.walkPattern(param);
949949
this.inScope(statement.params, () => {
950950
if(statement.body.type === "BlockStatement") {
951+
this.detectStrictMode(statement.body.body);
951952
this.prewalkStatement(statement.body);
952953
this.walkStatement(statement.body);
953954
} else {
@@ -1316,6 +1317,7 @@ class Parser extends Tapable {
13161317
this.walkPattern(param);
13171318
this.inScope(expression.params, () => {
13181319
if(expression.body.type === "BlockStatement") {
1320+
this.detectStrictMode(expression.body.body);
13191321
this.prewalkStatement(expression.body);
13201322
this.walkStatement(expression.body);
13211323
} else {
@@ -1329,6 +1331,7 @@ class Parser extends Tapable {
13291331
this.walkPattern(param);
13301332
this.inScope(expression.params, () => {
13311333
if(expression.body.type === "BlockStatement") {
1334+
this.detectStrictMode(expression.body.body);
13321335
this.prewalkStatement(expression.body);
13331336
this.walkStatement(expression.body);
13341337
} else {
@@ -1580,6 +1583,7 @@ class Parser extends Tapable {
15801583
this.scope = {
15811584
inTry: false,
15821585
inShorthand: false,
1586+
isStrict: oldScope.isStrict,
15831587
definitions: oldScope.definitions.createChild(),
15841588
renames: oldScope.renames.createChild()
15851589
};
@@ -1604,6 +1608,16 @@ class Parser extends Tapable {
16041608
this.scope = oldScope;
16051609
}
16061610

1611+
detectStrictMode(statements) {
1612+
const isStrict = statements.length >= 1 &&
1613+
statements[0].type === "ExpressionStatement" &&
1614+
statements[0].expression.type === "Literal" &&
1615+
statements[0].expression.value === "use strict";
1616+
if(isStrict) {
1617+
this.scope.isStrict = true;
1618+
}
1619+
}
1620+
16071621
enterPattern(pattern, onIdent) {
16081622
if(!pattern) return;
16091623
switch(pattern.type) {
@@ -1775,12 +1789,15 @@ class Parser extends Tapable {
17751789
const oldComments = this.comments;
17761790
this.scope = {
17771791
inTry: false,
1792+
inShorthand: false,
1793+
isStrict: false,
17781794
definitions: new StackedSetMap(),
17791795
renames: new StackedSetMap()
17801796
};
17811797
const state = this.state = initialState || {};
17821798
this.comments = comments;
17831799
if(this.hooks.program.call(ast, comments) === undefined) {
1800+
this.detectStrictMode(ast.body);
17841801
this.prewalkStatements(ast.body);
17851802
this.walkStatements(ast.body);
17861803
}

‎lib/UseStrictPlugin.js

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ class UseStrictPlugin {
1212
normalModuleFactory
1313
}) => {
1414
const handler = (parser) => {
15-
const parserInstance = parser;
1615
parser.hooks.program.tap("UseStrictPlugin", (ast) => {
1716
const firstNode = ast.body[0];
1817
if(firstNode &&
@@ -24,8 +23,8 @@ class UseStrictPlugin {
2423
// @see https://github.com/webpack/webpack/issues/1970
2524
const dep = new ConstDependency("", firstNode.range);
2625
dep.loc = firstNode.loc;
27-
parserInstance.state.current.addDependency(dep);
28-
parserInstance.state.module.buildInfo.strict = true;
26+
parser.state.current.addDependency(dep);
27+
parser.state.module.buildInfo.strict = true;
2928
}
3029
});
3130
};

‎lib/dependencies/HarmonyDetectionParserPlugin.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ module.exports = class HarmonyDetectionParserPlugin {
4343
};
4444
module.addDependency(initDep);
4545
parser.state.harmonyParserScope = parser.state.harmonyParserScope || {};
46+
parser.scope.isStrict = true;
4647
module.buildMeta.exportsType = "namespace";
4748
module.buildInfo.strict = true;
4849
module.buildInfo.exportsArgument = "__webpack_exports__";

‎test/configCases/parsing/issue-4857/index.js

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
it("should transpile unreachable branches", () => {
2+
let count = 0;
3+
4+
// BlockStatement
5+
if(true) {
6+
count++;
7+
} else {
8+
import("NOT_REACHABLE");
9+
}
10+
if(false) {
11+
import("NOT_REACHABLE");
12+
} else {
13+
count++;
14+
}
15+
16+
// ExpressionStatement
17+
if(true) count++;
18+
else import("NOT_REACHABLE");
19+
if(false) import("NOT_REACHABLE");
20+
else count++;
21+
22+
// ConditionalExpression
23+
true ? count++ : import("NOT_REACHABLE");
24+
false ? import("NOT_REACHABLE") : count++;
25+
26+
count.should.be.eql(6);
27+
});
28+
29+
it("should not remove hoisted variable declarations", () => {
30+
if(false) {
31+
var a, [,,b,] = [], {c, D: d, ["E"]: e} = {};
32+
var [{["_"]: f}, ...g] = [];
33+
}
34+
(() => {
35+
a;
36+
b;
37+
c;
38+
d;
39+
e;
40+
f;
41+
g;
42+
}).should.not.throw();
43+
});
44+
45+
it("should not remove hoisted function declarations in loose mode", () => {
46+
if(false) {
47+
function funcDecl() {}
48+
}
49+
(() => {
50+
funcDecl;
51+
}).should.not.throw();
52+
});
53+
54+
it("should remove hoisted function declarations in strict mode", () => {
55+
"use strict";
56+
if(false) {
57+
function funcDecl() {}
58+
}
59+
(() => {
60+
funcDecl;
61+
}).should.throw();
62+
});

‎test/configCases/parsing/issue-4857/webpack.config.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
module.exports = {
2+
optimization: {
3+
minimize: false
4+
}
5+
};

0 commit comments

Comments
 (0)