Fix bundler: stack overflow and circular imports (#1205)

swc_bundler:
 - Fix infinte loop (denoland/deno#8224)
 - Fix order of merging with circular imports. (Fixes denoland/deno#8246)
 - Fix detection of circular imports.
 - Fix logic of lca calculation.
 - Fix sorting algorithm.

bundler/Cargo.toml

@@ -8,7 +8,7 @@ edition = "2018"
 license = "Apache-2.0/MIT"
 name = "swc_bundler"
 repository = "https://github.com/swc-project/swc.git"
-version = "0.16.0"
+version = "0.16.1"
 
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 [features]

bundler/src/bundler/chunk/circular.rs

@@ -179,16 +179,21 @@ fn dependency_index<T>(item: &ModuleItem, deps: &[T]) -> Option<usize>
 where
     T: Borrow<ModuleItem>,
 {
-    let mut v = DepFinder { deps, idx: None };
+    let mut v = DepFinder {
+        deps,
+        idx: None,
+        last_usage_idx: None,
+    };
     item.visit_with(&Invalid { span: DUMMY_SP }, &mut v);
-    v.idx
+    v.idx.or(v.last_usage_idx)
 }
 
 struct DepFinder<'a, T>
 where
     T: Borrow<ModuleItem>,
 {
     deps: &'a [T],
+    last_usage_idx: Option<usize>,
     idx: Option<usize>,
 }
 
@@ -224,7 +229,11 @@ where
                     }
                 }
 
-                _ => {}
+                dep => {
+                    if DepUsageFinder::find(i, dep) {
+                        self.last_usage_idx = Some(idx);
+                    }
+                }
             }
         }
     }
@@ -253,3 +262,67 @@ where
     #[inline]
     fn visit_block_stmt(&mut self, _: &BlockStmt, _: &dyn Node) {}
 }
+
+/// Finds usage of `ident`
+struct DepUsageFinder<'a> {
+    ident: &'a Ident,
+    found: bool,
+}
+
+impl<'a> Visit for DepUsageFinder<'a> {
+    noop_visit_type!();
+
+    fn visit_call_expr(&mut self, e: &CallExpr, _: &dyn Node) {
+        if self.found {
+            return;
+        }
+
+        match &e.callee {
+            ExprOrSuper::Super(_) => {}
+            ExprOrSuper::Expr(callee) => match &**callee {
+                Expr::Ident(..) => {}
+                _ => {
+                    callee.visit_with(e, self);
+                }
+            },
+        }
+
+        e.args.visit_with(e, self);
+    }
+
+    fn visit_ident(&mut self, i: &Ident, _: &dyn Node) {
+        if self.found {
+            return;
+        }
+
+        if i.span.ctxt() == self.ident.span.ctxt() && i.sym == self.ident.sym {
+            self.found = true;
+        }
+    }
+
+    fn visit_member_expr(&mut self, e: &MemberExpr, _: &dyn Node) {
+        if self.found {
+            return;
+        }
+
+        e.obj.visit_with(e as _, self);
+
+        if e.computed {
+            e.prop.visit_with(e as _, self);
+        }
+    }
+}
+
+impl<'a> DepUsageFinder<'a> {
+    pub fn find<N>(ident: &'a Ident, node: &N) -> bool
+    where
+        N: VisitWith<Self>,
+    {
+        let mut v = DepUsageFinder {
+            ident,
+            found: false,
+        };
+        node.visit_with(&Invalid { span: DUMMY_SP } as _, &mut v);
+        v.found
+    }
+}

bundler/src/bundler/chunk/export.rs

@@ -184,19 +184,6 @@ where
 
                 if let Some(id) = id_of_export_namespace_from {
                     dep = self.wrap_esm_as_a_var(plan, dep, &imported, merged, id)?;
-
-                    let module_plan;
-                    let module_plan = match plan.normal.get(&info.id) {
-                        Some(v) => v,
-                        None => {
-                            module_plan = Default::default();
-                            &module_plan
-                        }
-                    };
-
-                    dep = self
-                        .merge_imports(plan, &module_plan, dep, &info, merged, false)
-                        .context("failed to merge imports")?;
                 } else {
                     dep = self.remark_exports(dep, src.ctxt, None, false);
                 }

bundler/src/bundler/chunk/merge.rs

@@ -299,6 +299,19 @@ where
             if dep_info.is_es6 {
                 // print_hygiene("entry: before injection", &self.cm, &entry);
 
+                if !is_direct {
+                    prepend_stmts(&mut entry.body, take(&mut dep.body).into_iter());
+
+                    log::debug!(
+                        "Merged {} into {} as a transitive es module",
+                        dep_info.fm.name,
+                        info.fm.name,
+                    );
+
+                    // print_hygiene("ES6", &self.cm, &entry);
+                    continue;
+                }
+
                 // Replace import statement / require with module body
                 let mut injector = Es6ModuleInjector {
                     imported: take(&mut dep.body),
@@ -323,19 +336,6 @@ where
                     continue;
                 }
 
-                if !is_direct {
-                    prepend_stmts(&mut entry.body, injector.imported.into_iter());
-
-                    log::debug!(
-                        "Merged {} into {} as a transitive es module",
-                        dep_info.fm.name,
-                        info.fm.name,
-                    );
-
-                    // print_hygiene("ES6", &self.cm, &entry);
-                    continue;
-                }
-
                 // print_hygiene("entry: failed to inject", &self.cm, &entry);
 
                 dep.body = take(&mut injector.imported);

bundler/src/bundler/chunk/plan/lca.rs

@@ -67,7 +67,7 @@ fn check_itself(b: &PlanBuilder, li: &[ModuleId], ri: &[ModuleId]) -> Option<Mod
         for &r in ri {
             // Root
             if g.neighbors_directed(r, Incoming).count() == 0 {
-                return Some(l);
+                return Some(r);
             }
 
             if l == r {

bundler/src/bundler/chunk/plan/mod.rs

@@ -11,7 +11,6 @@ use petgraph::{
     EdgeDirection::{Incoming, Outgoing},
 };
 use std::{
-    cmp::Ordering,
     collections::{hash_map::Entry, HashMap, HashSet},
     ops::{Deref, DerefMut},
 };
@@ -386,9 +385,7 @@ where
 
         // Sort transitive chunks topologically.
         for (_, normal_plan) in &mut plans.normal {
-            normal_plan
-                .transitive_chunks
-                .sort_by(|a, b| toposort(&builder, *a, *b));
+            toposort(&builder, &mut normal_plan.transitive_chunks);
         }
 
         // Handle circular imports
@@ -523,7 +520,12 @@ where
         }
 
         // Prevent dejavu
-        for (src, _) in &m.imports.specifiers {
+        for (src, _) in m
+            .imports
+            .specifiers
+            .iter()
+            .chain(m.exports.reexports.iter())
+        {
             if builder.all_deps.contains_key(&(src.module_id, module_id)) {
                 log::debug!("Circular dep: {:?} => {:?}", module_id, src.module_id);
 
@@ -548,16 +550,24 @@ where
     }
 }
 
-/// Compare topology of `i` and `k`.
-fn toposort(b: &PlanBuilder, i: ModuleId, j: ModuleId) -> Ordering {
-    //
-    let higher = least_common_ancestor(b, &[i, j]);
-
-    if higher == i {
-        Ordering::Greater
-    } else if higher == j {
-        Ordering::Less
-    } else {
-        Ordering::Equal
+fn toposort(b: &PlanBuilder, module_ids: &mut Vec<ModuleId>) {
+    let len = module_ids.len();
+
+    if module_ids.len() <= 1 {
+        return;
+    }
+
+    for i in 0..len {
+        for j in i..len {
+            let mi = module_ids[i];
+            let mj = module_ids[j];
+            if mi == mj {
+                continue;
+            }
+
+            if b.direct_deps.contains_edge(mj, mi) {
+                module_ids.swap(i, j);
+            }
+        }
     }
 }

bundler/tests/deno.rs

@@ -73,6 +73,11 @@ fn deno_8211() {
     run("https://unpkg.com/luxon@1.25.0/src/luxon.js", None);
 }
 
+#[test]
+fn deno_8246() {
+    run("https://raw.githubusercontent.com/nats-io/nats.deno/v1.0.0-11/nats-base-client/internal_mod.ts",None);
+}
+
 fn run(url: &str, expeceted_bytes: Option<usize>) {
     let dir = tempfile::tempdir().expect("failed to crate temp file");
     let path = dir.path().join("main.js");

spack/tests/pass/issue-1139/example-1/output/entry.js

@@ -1,8 +1,8 @@
 function f2() {
     console.log("f2");
 }
-f1();
 export function f1() {
     console.log("f1");
 }
+f1();
 f2();

spack/tests/pass/issue-1139/example-2/output/entry.js

@@ -1,8 +1,8 @@
 function f2() {
     console.log("f2");
 }
-f1();
 function f1() {
     console.log("f1");
 }
+f1();
 f2();

spack/tests/pass/transitive/import/simple-2/output/entry.js

@@ -1,9 +1,9 @@
-const common1 = 1;
 const common = 2;
 const common2 = common;
-console.log('a', common1, common2);
+const common1 = 1;
 const common3 = 3;
 const common31 = common3;
+console.log('a', common1, common2);
 console.log('b', common31, common1);
 var common4;
 try {