Repository: vercel/next.js
Pull request: [turbopack] Fix a small issue in the analyzer where we wouldn't skip assignments to free vars that were just identifiers

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.


Closes PACK-5222

Changed files:
filename: turbopack/crates/turbopack-ecmascript/src/analyzer/mod.rs
patch: @@ -2042,11 +2042,11 @@ impl JsValue {
     /// Returns any matching defined replacement that matches this value (the replacement that
     /// matches `$self.$prop`).
     ///
-    /// Optionally when passed a VarGraph, verifies that the first segment is not a local
+    /// Uses the `VarGraph` to verify that the first segment is not a local
     /// variable/was not reassigned.
     pub fn match_free_var_reference<'a, T>(
         &self,
-        var_graph: Option<&VarGraph>,
+        var_graph: &VarGraph,
         free_var_references: &'a FxIndexMap<
             DefinableNameSegment,
             FxIndexMap<Vec<DefinableNameSegment>, T>,
@@ -2063,9 +2063,7 @@ impl JsValue {
 
                 let name_rev_it = name.iter().map(Cow::Borrowed).rev();
                 if name_rev_it.eq(self.iter_definable_name_rev()) {
-                    if let Some(var_graph) = var_graph
-                        && let DefinableNameSegment::Name(first_str) = name.first().unwrap()
-                    {
+                    if let DefinableNameSegment::Name(first_str) = name.first().unwrap() {
                         let first_str: &str = first_str;
                         if var_graph
                             .free_var_ids

filename: turbopack/crates/turbopack-ecmascript/src/references/mod.rs
patch: @@ -52,7 +52,9 @@ use swc_core::{
         utils::IsDirective,
         visit::{
             AstParentKind, AstParentNodeRef, VisitAstPath, VisitWithAstPath,
-            fields::{AssignExprField, AssignTargetField, SimpleAssignTargetField},
+            fields::{
+                AssignExprField, AssignTargetField, BindingIdentField, SimpleAssignTargetField,
+            },
         },
     },
 };
@@ -2485,7 +2487,7 @@ async fn handle_typeof(
     analysis: &mut AnalyzeEcmascriptModuleResultBuilder,
 ) -> Result<()> {
     if let Some(value) = arg.match_free_var_reference(
-        Some(state.var_graph),
+        state.var_graph,
         &*state.free_var_references,
         &DefinableNameSegment::TypeOf,
     ) {
@@ -2533,11 +2535,20 @@ async fn handle_free_var_reference(
     // We don't want to replace assignments as this would lead to invalid code.
     if matches!(
         ast_path,
+        // Matches assignments to members
         [
             ..,
             AstParentKind::AssignExpr(AssignExprField::Left),
             AstParentKind::AssignTarget(AssignTargetField::Simple),
             AstParentKind::SimpleAssignTarget(SimpleAssignTargetField::Member),
+        ] |
+        // Matches assignments to identifiers
+        [
+            ..,
+            AstParentKind::AssignExpr(AssignExprField::Left),
+            AstParentKind::AssignTarget(AssignTargetField::Simple),
+            AstParentKind::SimpleAssignTarget(SimpleAssignTargetField::Ident),
+            AstParentKind::BindingIdent(BindingIdentField::Id),
         ]
     ) {
         return Ok(false);
@@ -2887,7 +2898,7 @@ async fn value_visitor_inner(
         let compile_time_info = compile_time_info.await?;
         if let JsValue::TypeOf(_, arg) = &v
             && let Some(value) = arg.match_free_var_reference(
-                Some(var_graph),
+                var_graph,
                 free_var_references,
                 &DefinableNameSegment::TypeOf,
             )