Skip to content

Commit 74a0aa2

Browse files
authored
[turbopack] Fix a small issue in the analyzer where we wouldn't skip assignments to free vars that were just identifiers (#82392)
Correctly skip assignments when replacing free variables Drop an `Option<..>` for a parameter that is always supplied. These are fixes i tripped over when attempting to compile time replace `module` and `exports`. That approach failed (see #82285 for what happened instead), but we can keep these cleanups.
1 parent 2c3e9d8 commit 74a0aa2

File tree

2 files changed

+17
-8
lines changed
  • turbopack/crates/turbopack-ecmascript/src

2 files changed

+17
-8
lines changed

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

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2042,11 +2042,11 @@ impl JsValue {
20422042
/// Returns any matching defined replacement that matches this value (the replacement that
20432043
/// matches `$self.$prop`).
20442044
///
2045-
/// Optionally when passed a VarGraph, verifies that the first segment is not a local
2045+
/// Uses the `VarGraph` to verify that the first segment is not a local
20462046
/// variable/was not reassigned.
20472047
pub fn match_free_var_reference<'a, T>(
20482048
&self,
2049-
var_graph: Option<&VarGraph>,
2049+
var_graph: &VarGraph,
20502050
free_var_references: &'a FxIndexMap<
20512051
DefinableNameSegment,
20522052
FxIndexMap<Vec<DefinableNameSegment>, T>,
@@ -2063,9 +2063,7 @@ impl JsValue {
20632063

20642064
let name_rev_it = name.iter().map(Cow::Borrowed).rev();
20652065
if name_rev_it.eq(self.iter_definable_name_rev()) {
2066-
if let Some(var_graph) = var_graph
2067-
&& let DefinableNameSegment::Name(first_str) = name.first().unwrap()
2068-
{
2066+
if let DefinableNameSegment::Name(first_str) = name.first().unwrap() {
20692067
let first_str: &str = first_str;
20702068
if var_graph
20712069
.free_var_ids

turbopack/crates/turbopack-ecmascript/src/references/mod.rs

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,9 @@ use swc_core::{
5252
utils::IsDirective,
5353
visit::{
5454
AstParentKind, AstParentNodeRef, VisitAstPath, VisitWithAstPath,
55-
fields::{AssignExprField, AssignTargetField, SimpleAssignTargetField},
55+
fields::{
56+
AssignExprField, AssignTargetField, BindingIdentField, SimpleAssignTargetField,
57+
},
5658
},
5759
},
5860
};
@@ -2485,7 +2487,7 @@ async fn handle_typeof(
24852487
analysis: &mut AnalyzeEcmascriptModuleResultBuilder,
24862488
) -> Result<()> {
24872489
if let Some(value) = arg.match_free_var_reference(
2488-
Some(state.var_graph),
2490+
state.var_graph,
24892491
&*state.free_var_references,
24902492
&DefinableNameSegment::TypeOf,
24912493
) {
@@ -2533,11 +2535,20 @@ async fn handle_free_var_reference(
25332535
// We don't want to replace assignments as this would lead to invalid code.
25342536
if matches!(
25352537
ast_path,
2538+
// Matches assignments to members
25362539
[
25372540
..,
25382541
AstParentKind::AssignExpr(AssignExprField::Left),
25392542
AstParentKind::AssignTarget(AssignTargetField::Simple),
25402543
AstParentKind::SimpleAssignTarget(SimpleAssignTargetField::Member),
2544+
] |
2545+
// Matches assignments to identifiers
2546+
[
2547+
..,
2548+
AstParentKind::AssignExpr(AssignExprField::Left),
2549+
AstParentKind::AssignTarget(AssignTargetField::Simple),
2550+
AstParentKind::SimpleAssignTarget(SimpleAssignTargetField::Ident),
2551+
AstParentKind::BindingIdent(BindingIdentField::Id),
25412552
]
25422553
) {
25432554
return Ok(false);
@@ -2887,7 +2898,7 @@ async fn value_visitor_inner(
28872898
let compile_time_info = compile_time_info.await?;
28882899
if let JsValue::TypeOf(_, arg) = &v
28892900
&& let Some(value) = arg.match_free_var_reference(
2890-
Some(var_graph),
2901+
var_graph,
28912902
free_var_references,
28922903
&DefinableNameSegment::TypeOf,
28932904
)

0 commit comments

Comments
 (0)