Skip to content

Commit 920b48d

Browse files
authored
fix(turbopack): Consider scoping of var declarations (#77954)
### What? Fix turbopack es analyzer, for the case below ```js const shouldRun = () => false; export default function run() { if (shouldRun()) { var x = true; } if (x) { return 'should not run'; } return 'should run'; } ``` ### Why? ### How? Fixes #77126 Closes PACK-4159
1 parent 66ecb87 commit 920b48d

25 files changed

+3822
-6079
lines changed

turbopack/crates/turbopack-ecmascript/src/analyzer/graph.rs

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1650,7 +1650,27 @@ impl VisitAstPath for Analyzer<'_> {
16501650
) {
16511651
if self.var_decl_kind.is_some() {
16521652
if let Some(init) = &n.init {
1653-
self.current_value = Some(self.eval_context.eval(init));
1653+
// For case like
1654+
//
1655+
// if (shouldRun()) {
1656+
// var x = true;
1657+
// }
1658+
// if (x) {
1659+
// }
1660+
//
1661+
// The variable `x` is undefined
1662+
1663+
let should_include_undefined = matches!(self.var_decl_kind, Some(VarDeclKind::Var))
1664+
&& is_lexically_block_scope(ast_path);
1665+
let init_value = self.eval_context.eval(init);
1666+
self.current_value = Some(if should_include_undefined {
1667+
JsValue::alternatives(vec![
1668+
init_value,
1669+
JsValue::Constant(ConstantValue::Undefined),
1670+
])
1671+
} else {
1672+
init_value
1673+
});
16541674
}
16551675
}
16561676
{
@@ -2185,6 +2205,23 @@ impl VisitAstPath for Analyzer<'_> {
21852205
}
21862206
}
21872207

2208+
fn is_lexically_block_scope(ast_path: &mut AstNodePath<AstParentNodeRef>) -> bool {
2209+
let mut iter = ast_path.iter().rev().peekable();
2210+
2211+
while let Some(cur) = iter.next() {
2212+
// If it's a block statement, we need to check if it's Function#body
2213+
if matches!(cur.kind(), AstParentKind::BlockStmt(..)) {
2214+
if let Some(next) = iter.peek() {
2215+
return !matches!(next.kind(), AstParentKind::Function(FunctionField::Body));
2216+
}
2217+
return false;
2218+
}
2219+
}
2220+
2221+
// This `var` is not in a block scope
2222+
false
2223+
}
2224+
21882225
impl Analyzer<'_> {
21892226
fn add_conditional_if_effect_with_early_return(
21902227
&mut self,

turbopack/crates/turbopack-ecmascript/tests/analyzer/graph/iife-2/graph-explained.snapshot

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ i = (0 | ???*0*)
44
- *0* updated with update expression
55
⚠️ This value might have side effects
66

7-
key = keys[i]
7+
key = (keys[i] | undefined)
88

99
keys = FreeVar(Object)["keys"](namespace)
1010

turbopack/crates/turbopack-ecmascript/tests/analyzer/graph/iife-2/graph.snapshot

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -35,21 +35,30 @@
3535
),
3636
(
3737
"key",
38-
Member(
39-
3,
40-
Variable(
41-
(
42-
"keys",
43-
#4,
38+
Alternatives {
39+
total_nodes: 5,
40+
values: [
41+
Member(
42+
3,
43+
Variable(
44+
(
45+
"keys",
46+
#4,
47+
),
48+
),
49+
Variable(
50+
(
51+
"i",
52+
#4,
53+
),
54+
),
4455
),
45-
),
46-
Variable(
47-
(
48-
"i",
49-
#4,
56+
Constant(
57+
Undefined,
5058
),
51-
),
52-
),
59+
],
60+
logical_property: None,
61+
},
5362
),
5463
(
5564
"keys",

turbopack/crates/turbopack-ecmascript/tests/analyzer/graph/iife-2/resolved-effects.snapshot

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,14 +24,14 @@
2424
- *0* arguments[1]
2525
⚠️ function calls are not analysed yet
2626

27-
0 -> 14 call = (...) => parent(???*0*, (???*2* ? ???*3* : ???*10*), (???*15* | {}){truthy})
27+
0 -> 14 call = (...) => parent(???*0*, (???*2* ? ???*3* : (???*10* | undefined)), (???*15* | {}){truthy})
2828
- *0* ???*1*[key]
2929
⚠️ unknown object
3030
- *1* arguments[0]
3131
⚠️ function calls are not analysed yet
3232
- *2* arguments[1]
3333
⚠️ function calls are not analysed yet
34-
- *3* `${???*4*}.${???*5*}`
34+
- *3* `${???*4*}.${(???*5* | undefined)}`
3535
⚠️ nested operation
3636
- *4* arguments[1]
3737
⚠️ function calls are not analysed yet

turbopack/crates/turbopack-ecmascript/tests/analyzer/graph/iife-2/resolved-explained.snapshot

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ i = (0 | ???*0*)
44
- *0* updated with update expression
55
⚠️ This value might have side effects
66

7-
key = ???*0*
7+
key = (???*0* | undefined)
88
- *0* ???*1*[i]
99
⚠️ unknown object
1010
⚠️ This value might have side effects

0 commit comments

Comments
 (0)