Repository: swc-project/swc (rust)

Issue:
Issues with lazy requires codegen when using nested closures
### Describe the bug

SWC generates faulty code when using `module.lazy = true` and `module.type = "commonjs"` along with nested closures.

It will generate a mix of lazy and non-lazy code:

```js
import { spawn } from "child_process";

function log() {
  console.log(spawn);
}

const other = () => {
  const nestedClosure = () => {
    spawn("ls");
  };
};
```

Generates 

```js
"use strict";
var _childProcess = require("child_process");
function log() {
    console.log(_childProcess().spawn);
}
const other = ()=>{
    const nestedClosure = ()=>{
        (0, _childProcess).spawn("ls");
    };
};
```

Notice that the `require` statement and the call in `nestedClosure` are non-lazy, but the call in `function log()` is `lazy`.

## Ways to not get the problem

### Inverting the functions

Inverting the order of the `log` and `other` functions will not produce the same issue. Instead, `swc` will generate completely non-lazy, but functional code.

```js
import { spawn } from "child_process";

const other = () => {
  const nestedClosure = () => {
    spawn("ls");
  };
};

function log() {
  console.log(spawn);
}
```

Generates

```js
"use strict";
var _childProcess = require("child_process");
const other = ()=>{
    const nestedClosure = ()=>{
        (0, _childProcess).spawn("ls");
    };
};
function log() {
    console.log(_childProcess.spawn);
}
```

### Avoid using nested closures

Replacing either `other` or `nestedClosure` with a named function will also produce valid, lazy code:

```js
import { spawn } from "child_process";

function log() {
  console.log(spawn);
}

function other() {
  const nestedClosure = () => {
    spawn("ls");
  };
};
```

Generates

```js
"use strict";
function _childProcess() {
    const data = require("child_process");
    _childProcess = function() {
        return data;
    };
    return data;
}
function log() {
    console.log(_childProcess().spawn);
}
function other() {
    const nestedClosure = ()=>{
        _childProcess().spawn("ls");
    };
}
```

### Input code

```typescript
import { spawn } from "child_process";

function log() {
  console.log(spawn);
}

const other = () => {
  const nestedClosure = () => {
    spawn("ls");
  };
};
```


### Config

```json
{
  "jsc": {
    "parser": {
      "syntax": "typescript"
    },
    "target": "es2020"
  },
  "module": {
    "type": "commonjs",
    "lazy": true
  }
}
```


### Playground link

https://play.swc.rs/?version=1.2.115&code=H4sIAAAAAAAAA1WOMQrDMBAE%2B3vFospu8gHhNHlICPI5Fsg6oTuTwujvkRMIpN2dHTZuRarhgJbHK6NhqbLBhTWm%2BV6qBFZ1nmjZc7AoGUmew4iDgCBZJfHlTD7r0VMjOmOD2MoVEzo7XX%2B4IbMaz7ckulf%2B6%2FG9MLikrpuA1nWe3kTS5YKhAAAA&config=H4sIAAAAAAAAA0WNSQ6DMAxF7%2BI1iypL7tBDWKlBQWSQbaSmEXevg4LY2X94v8EmHuYGBVmI%2ByU1KX5hBq2FxHMoChOomLTgLnTag7ySWoTEvZwze89ZaAQmiCGFpXaYz7EwiTwWpnW%2Fk6exYv4cXWjXnjGtEnPapFPxZxTlo4%2Fe0AEK8h7Ny%2F8DWryisckAAAA%3D

### Expected behavior

I expect the output to produce a lazy `require` statement, along with lazy `_childProcess()` calls.

```js
"use strict";
function _childProcess() {
    const data = require("child_process");
    _childProcess = function() {
        return data;
    };
    return data;
}
function log() {
    console.log(_childProcess().spawn);
}
const other = ()=>{
    const nestedClosure = () => {
        _childProcess().spawn("ls");
    }
};
```

### Version

@swc/cli: 0.1.55, @swc/core: 1.2.121

### Additional context

_No response_

Pull request:
Fix #3081 and lazy imports
## Description

Fixes #3081 

Lazy imports generate broken code if an usage of the import precedes the blacklisting of the identifier. 

The ultimate cause was that the blacklisting of identifiers was done as the generated statements get emitted, causing statements prior to be emitted as lazy, even if an identifier was subsequently blacklisted.

A proximal cause of the first identified issue (dealing with nested closures) was an unidentified issue in tracking the top-level, and nested-closures believing they were top-level.

## Background

An imported identifier are (currently) blacklisted for two reasons:
1. It is being re-exported
2. It is used at the top level

These two reasons can be generalized as only being used at the top-level, since that's also what exports are.

## Implementation

To solve the problem, we must perform two additional prepasses on the AST:
- First, populate a `Scope` with imports, to be able to map identifiers to
  their sources
- Second, navigate all top-level identifiers and mark them as non-lazy

The second pass is performed using a new `Visitor` which only visits top-level nodes.

**Related issue:** #3081


Patch:
diff --git a/crates/swc_ecma_transforms_module/src/amd.rs b/crates/swc_ecma_transforms_module/src/amd.rs
index 19d4940446..53dfc08da3 100644
--- a/crates/swc_ecma_transforms_module/src/amd.rs
+++ b/crates/swc_ecma_transforms_module/src/amd.rs
@@ -647,10 +647,7 @@ where
 
     fn fold_prop(&mut self, p: Prop) -> Prop {
         match p {
-            Prop::Shorthand(ident) => {
-                let top_level = self.in_top_level;
-                Scope::fold_shorthand_prop(self, top_level, ident)
-            }
+            Prop::Shorthand(ident) => Scope::fold_shorthand_prop(self, ident),
 
             _ => p.fold_children_with(self),
         }
diff --git a/crates/swc_ecma_transforms_module/src/common_js.rs b/crates/swc_ecma_transforms_module/src/common_js.rs
index 37245a2428..8bb046aa27 100644
--- a/crates/swc_ecma_transforms_module/src/common_js.rs
+++ b/crates/swc_ecma_transforms_module/src/common_js.rs
@@ -10,14 +10,17 @@ use std::{
     rc::Rc,
 };
 use swc_atoms::{js_word, JsWord};
-use swc_common::{collections::AHashMap, FileName, Mark, Span, DUMMY_SP};
+use swc_common::{
+    collections::{AHashMap, AHashSet},
+    FileName, Mark, Span, DUMMY_SP,
+};
 use swc_ecma_ast::*;
 use swc_ecma_transforms_base::helper;
 use swc_ecma_utils::{
     ident::IdentLike, member_expr, private_ident, quote_ident, quote_str, var::VarCollector,
     DestructuringFinder, ExprFactory, IsDirective,
 };
-use swc_ecma_visit::{noop_fold_type, Fold, FoldWith, VisitWith};
+use swc_ecma_visit::{noop_fold_type, noop_visit_type, Fold, FoldWith, Visit, VisitWith};
 
 pub fn common_js(
     top_level_mark: Mark,
@@ -55,6 +58,73 @@ where
     }
 }
 
+struct LazyIdentifierVisitor {
+    scope: Rc<RefCell<Scope>>,
+    top_level_idents: AHashSet<JsWord>,
+}
+impl LazyIdentifierVisitor {
+    fn new(scope: Rc<RefCell<Scope>>) -> Self {
+        LazyIdentifierVisitor {
+            scope,
+            top_level_idents: Default::default(),
+        }
+    }
+}
+
+/*
+An import can be performed lazily if it isn't used at the module's top-level.
+This visitor scans only the identifiers at the top level by not traversing nodes
+that introduce nesting. It then looks up identifiers to see if they match an
+imported specifier name.
+ */
+impl Visit for LazyIdentifierVisitor {
+    noop_visit_type!();
+
+    fn visit_import_decl(&mut self, _: &ImportDecl) {}
+    fn visit_export_decl(&mut self, _: &ExportDecl) {}
+    fn visit_named_export(&mut self, _: &NamedExport) {}
+    fn visit_export_default_decl(&mut self, _: &ExportDefaultDecl) {}
+    fn visit_export_default_expr(&mut self, _: &ExportDefaultExpr) {}
+
+    fn visit_export_all(&mut self, export: &ExportAll) {
+        self.top_level_idents.insert(export.src.value.clone());
+    }
+
+    fn visit_labeled_stmt(&mut self, _: &LabeledStmt) {}
+    fn visit_continue_stmt(&mut self, _: &ContinueStmt) {}
+
+    fn visit_arrow_expr(&mut self, _: &ArrowExpr) {}
+    fn visit_function(&mut self, _: &Function) {}
+    fn visit_constructor(&mut self, _: &Constructor) {}
+    fn visit_setter_prop(&mut self, _: &SetterProp) {}
+    fn visit_getter_prop(&mut self, _: &GetterProp) {}
+    fn visit_class_prop(&mut self, _: &ClassProp) {}
+
+    fn visit_prop_name(&mut self, prop_name: &PropName) {
+        match prop_name {
+            PropName::Computed(n) => n.visit_with(self),
+            _ => {}
+        }
+    }
+
+    fn visit_decl(&mut self, decl: &Decl) {
+        match decl {
+            Decl::Class(ref c) => {
+                c.class.super_class.visit_with(self);
+                c.class.body.visit_with(self);
+            }
+            _ => {}
+        }
+    }
+
+    fn visit_ident(&mut self, ident: &Ident) {
+        let v = self.scope.borrow().idents.get(&ident.to_id()).cloned();
+        if let Some((src, _)) = v {
+            self.top_level_idents.insert(src.clone());
+        }
+    }
+}
+
 struct CommonJs<P>
 where
     P: ImportResolver,
@@ -89,7 +159,7 @@ where
         // Used only if export * exists
         let mut exported_names: Option<Ident> = None;
 
-        // make a preliminary pass through to collect exported names ahead of time
+        // Make a preliminary pass through to collect exported names ahead of time
         for item in &items {
             match item {
                 ModuleItem::ModuleDecl(ModuleDecl::ExportNamed(NamedExport {
@@ -125,6 +195,26 @@ where
             }
         }
 
+        // Make another preliminary pass to collect all import sources and their
+        // specifiers.
+        for item in &items {
+            self.in_top_level = true;
+            match item {
+                ModuleItem::ModuleDecl(ModuleDecl::Import(import)) => {
+                    self.scope.borrow_mut().insert_import(import.clone())
+                }
+                _ => {}
+            }
+        }
+
+        // Map all top-level identifiers that match imported specifiers, and blacklist
+        // them from lazy imports.
+        let mut visitor = LazyIdentifierVisitor::new(self.scope.clone());
+        items.visit_with(&mut visitor);
+        for ident in visitor.top_level_idents {
+            self.scope.borrow_mut().lazy_blacklist.insert(ident);
+        }
+
         for item in items {
             self.in_top_level = true;
 
@@ -204,11 +294,6 @@ where
 
                     match item {
                         ModuleItem::ModuleDecl(ModuleDecl::ExportAll(export)) => {
-                            self.scope
-                                .borrow_mut()
-                                .lazy_blacklist
-                                .insert(export.src.value.clone());
-
                             let mut scope_ref_mut = self.scope.borrow_mut();
                             let scope = &mut *scope_ref_mut;
 
@@ -776,10 +861,7 @@ where
 
     fn fold_prop(&mut self, p: Prop) -> Prop {
         match p {
-            Prop::Shorthand(ident) => {
-                let top_level = self.in_top_level;
-                Scope::fold_shorthand_prop(self, top_level, ident)
-            }
+            Prop::Shorthand(ident) => Scope::fold_shorthand_prop(self, ident),
 
             _ => p.fold_children_with(self),
         }
diff --git a/crates/swc_ecma_transforms_module/src/umd.rs b/crates/swc_ecma_transforms_module/src/umd.rs
index 805c1917fe..63673b3701 100644
--- a/crates/swc_ecma_transforms_module/src/umd.rs
+++ b/crates/swc_ecma_transforms_module/src/umd.rs
@@ -788,10 +788,7 @@ where
 
     fn fold_prop(&mut self, p: Prop) -> Prop {
         match p {
-            Prop::Shorthand(ident) => {
-                let top_level = self.in_top_level;
-                Scope::fold_shorthand_prop(self, top_level, ident)
-            }
+            Prop::Shorthand(ident) => Scope::fold_shorthand_prop(self, ident),
 
             _ => p.fold_children_with(self),
         }
diff --git a/crates/swc_ecma_transforms_module/src/util.rs b/crates/swc_ecma_transforms_module/src/util.rs
index 09868ef2c7..1a5843f913 100644
--- a/crates/swc_ecma_transforms_module/src/util.rs
+++ b/crates/swc_ecma_transforms_module/src/util.rs
@@ -424,13 +424,9 @@ impl Scope {
         }
     }
 
-    pub(super) fn fold_shorthand_prop(
-        folder: &mut impl ModulePass,
-        top_level: bool,
-        prop: Ident,
-    ) -> Prop {
+    pub(super) fn fold_shorthand_prop(folder: &mut impl ModulePass, prop: Ident) -> Prop {
         let key = prop.clone();
-        let value = Scope::fold_ident(folder, top_level, prop);
+        let value = Scope::fold_ident(folder, prop);
         match value {
             Ok(value) => Prop::KeyValue(KeyValueProp {
                 key: PropName::Ident(key),
@@ -440,15 +436,11 @@ impl Scope {
         }
     }
 
-    fn fold_ident(folder: &mut impl ModulePass, top_level: bool, i: Ident) -> Result<Expr, Ident> {
+    fn fold_ident(folder: &mut impl ModulePass, i: Ident) -> Result<Expr, Ident> {
         let v = folder.scope().idents.get(&i.to_id()).cloned();
         match v {
             None => Err(i),
             Some((src, prop)) => {
-                if top_level {
-                    folder.scope_mut().lazy_blacklist.insert(src.clone());
-                }
-
                 let lazy = if folder.scope().lazy_blacklist.contains(&src) {
                     false
                 } else {
@@ -511,7 +503,7 @@ impl Scope {
         match expr {
             // In a JavaScript module, this is undefined at the top level (i.e., outside functions).
             Expr::This(ThisExpr { span }) if top_level => *undefined(span),
-            Expr::Ident(i) => match Self::fold_ident(folder, top_level, i) {
+            Expr::Ident(i) => match Self::fold_ident(folder, i) {
                 Ok(expr) => expr,
                 Err(ident) => Expr::Ident(ident),
             },
@@ -538,7 +530,7 @@ impl Scope {
             }) => {
                 let callee = if let Callee::Expr(expr) = callee {
                     let callee = if let Expr::Ident(ident) = *expr {
-                        match Self::fold_ident(folder, top_level, ident) {
+                        match Self::fold_ident(folder, ident) {
                             Ok(mut expr) => {
                                 if let Expr::Member(member) = &mut expr {
                                     if let Expr::Ident(ident) = member.obj.as_mut() {
@@ -712,7 +704,7 @@ impl Scope {
 
                         let left = if let PatOrExpr::Pat(ref left_pat) = expr.left {
                             if let Pat::Ident(BindingIdent { ref id, .. }) = **left_pat {
-                                let expr = match Self::fold_ident(folder, top_level, id.clone()) {
+                                let expr = match Self::fold_ident(folder, id.clone()) {
                                     Ok(expr) => expr,
                                     Err(ident) => Expr::Ident(ident),
                                 };


Test patch:
diff --git a/crates/swc_ecma_transforms_module/tests/common_js.rs b/crates/swc_ecma_transforms_module/tests/common_js.rs
index 63f8cf0ff5..58e355c788 100644
--- a/crates/swc_ecma_transforms_module/tests/common_js.rs
+++ b/crates/swc_ecma_transforms_module/tests/common_js.rs
@@ -2550,6 +2550,146 @@ require("black");
 "#
 );
 
+// lazy_issue_3081_1
+test!(
+    syntax(),
+    |_| tr(Config {
+        lazy: Lazy::Bool(true),
+        ..Default::default()
+    }),
+    lazy_issue_3081_1,
+    r#"
+import { spawn } from "child_process";
+
+function log() { console.log(spawn); }
+
+const other = () => {
+  const nestedClosure = () => { spawn("ls"); };
+}
+"#,
+    r#"
+"use strict";
+function _childProcess() {
+    const data = require("child_process");
+    _childProcess = function() {
+        return data;
+    };
+    return data;
+}
+function log() {
+    console.log(_childProcess().spawn);
+}
+const other = ()=>{
+    const nestedClosure = () => {
+        _childProcess().spawn("ls");
+    }
+}
+"#
+);
+
+// lazy_issue_3081_2
+test!(
+    syntax(),
+    |_| tr(Config {
+        lazy: Lazy::Bool(true),
+        ..Default::default()
+    }),
+    lazy_issue_3081_2,
+    r#"
+import { fn, Klass } from "lib";
+
+function myFn() {
+    fn()
+}
+class MyClass extends Klass {}
+"#,
+    r#"
+"use strict";
+var _lib = require("lib");
+function myFn() {
+    (0, _lib).fn();
+}
+class MyClass extends _lib.Klass {
+}
+"#
+);
+
+// lazy_computed_prop_name
+test!(
+    syntax(),
+    |_| tr(Config {
+        lazy: Lazy::Bool(true),
+        ..Default::default()
+    }),
+    lazy_computed_prop_name,
+    r#"
+import { x } from "libx";
+import { y } from "liby";
+
+class F {
+  get[x]() { }
+  get y() { y() }
+}
+"#,
+    r#"
+"use strict";
+var _libx = require("libx");
+function _liby() {
+    const data = require("liby");
+    _liby = function() {
+        return data;
+    };
+    return data;
+}
+
+class F {
+  get[_libx.x]() { }
+  get y() { _liby().y(); }
+}
+"#
+);
+
+// lazy_not_shadowed_by_labels
+test!(
+    syntax(),
+    |_| tr(Config {
+        lazy: Lazy::Bool(true),
+        ..Default::default()
+    }),
+    lazy_not_shadowed_by_labels,
+    r#"
+import { x } from "lib";
+
+function fn() {
+    x()
+}
+
+x:
+console.log(1);
+
+continue x;
+
+"#,
+    r#"
+"use strict";
+function _lib() {
+    const data = require("lib");
+    _lib = function() {
+        return data;
+    };
+    return data;
+}
+
+function fn() {
+    _lib().x();
+}
+x:
+console.log(1);
+
+continue x;
+"#
+);
+
 // lazy_dep_reexport_namespace
 test!(
     syntax(),