Apply unresolved mark to inserted undefined identifiers (#8151)

parcel-bundler · May 27, 2022 · 9a5d48d · 9a5d48d
1 parent f2d0a3a
commit 9a5d48d
Show file tree

Hide file tree

Showing 14 changed files with 101 additions and 65 deletions.
diff --git a/packages/core/integration-tests/test/integration/env-unused-require/index.js b/packages/core/integration-tests/test/integration/env-unused-require/index.js
@@ -0,0 +1,7 @@
+module.exports = function () {
+  if(process.env.ABC === 'a') {
+    return require("./unused.js");
+  } else {
+    return "ok";
+  }
+};
diff --git a/packages/core/integration-tests/test/integration/env-unused-require/unused.js b/packages/core/integration-tests/test/integration/env-unused-require/unused.js
@@ -0,0 +1 @@
+module.exports = "unused";
diff --git a/packages/core/integration-tests/test/javascript.js b/packages/core/integration-tests/test/javascript.js
@@ -347,6 +347,28 @@ describe('javascript', function () {
     assert(!contents.includes('import'));
   });
 
+  it('should ignore unused requires after process.env inlining', async function () {
+    let b = await bundle(
+      path.join(__dirname, '/integration/env-unused-require/index.js'),
+      {
+        env: {ABC: 'XYZ'},
+      },
+    );
+
+    assertBundles(b, [
+      {
+        type: 'js',
+        assets: ['index.js'],
+      },
+    ]);
+
+    let contents = await outputFS.readFile(b.getBundles()[0].filePath, 'utf8');
+    assert(!contents.includes('unused'));
+
+    let output = await run(b);
+    assert.strictEqual(output(), 'ok');
+  });
+
   it('should produce a basic JS bundle with object rest spread support', async function () {
     let b = await bundle(
       path.join(

diff --git a/packages/transformers/js/core/src/decl_collector.rs b/packages/transformers/js/core/src/decl_collector.rs
@@ -1,14 +1,12 @@
 use std::collections::HashSet;
 
-use swc_atoms::JsWord;
-use swc_common::SyntaxContext;
-use swc_ecmascript::ast;
+use swc_ecmascript::ast::{self, Id};
 use swc_ecmascript::visit::{Visit, VisitWith};
 
 /// This pass collects all declarations in a module into a single HashSet of tuples
 /// containing identifier names and their associated syntax context (scope).
 /// This is used later to determine whether an identifier references a declared variable.
-pub fn collect_decls(module: &ast::Module) -> HashSet<(JsWord, SyntaxContext)> {
+pub fn collect_decls(module: &ast::Module) -> HashSet<Id> {
   let mut c = DeclCollector {
     decls: HashSet::new(),
     in_var: false,
@@ -18,7 +16,7 @@ pub fn collect_decls(module: &ast::Module) -> HashSet<(JsWord, SyntaxContext)> {
 }
 
 struct DeclCollector {
-  decls: HashSet<(JsWord, SyntaxContext)>,
+  decls: HashSet<Id>,
   in_var: bool,
 }
 

diff --git a/packages/transformers/js/core/src/dependency_collector.rs b/packages/transformers/js/core/src/dependency_collector.rs
@@ -5,8 +5,8 @@ use std::path::Path;
 
 use serde::{Deserialize, Serialize};
 use swc_atoms::JsWord;
-use swc_common::{Mark, SourceMap, Span, SyntaxContext, DUMMY_SP};
-use swc_ecmascript::ast::{self, Callee, MemberProp};
+use swc_common::{Mark, SourceMap, Span, DUMMY_SP};
+use swc_ecmascript::ast::{self, Callee, Id, MemberProp};
 use swc_ecmascript::visit::{Fold, FoldWith};
 
 use crate::fold_member_expr_skip_prop;
@@ -59,8 +59,9 @@ pub struct DependencyDescriptor {
 pub fn dependency_collector<'a>(
   source_map: &'a SourceMap,
   items: &'a mut Vec<DependencyDescriptor>,
-  decls: &'a HashSet<(JsWord, SyntaxContext)>,
+  decls: &'a HashSet<Id>,
   ignore_mark: swc_common::Mark,
+  unresolved_mark: swc_common::Mark,
   config: &'a Config,
   diagnostics: &'a mut Vec<Diagnostic>,
 ) -> impl Fold + 'a {
@@ -72,6 +73,7 @@ pub fn dependency_collector<'a>(
     require_node: None,
     decls,
     ignore_mark,
+    unresolved_mark,
     config,
     diagnostics,
     import_meta: None,
@@ -84,8 +86,9 @@ struct DependencyCollector<'a> {
   in_try: bool,
   in_promise: bool,
   require_node: Option<ast::CallExpr>,
-  decls: &'a HashSet<(JsWord, SyntaxContext)>,
+  decls: &'a HashSet<Id>,
   ignore_mark: swc_common::Mark,
+  unresolved_mark: swc_common::Mark,
   config: &'a Config,
   diagnostics: &'a mut Vec<Diagnostic>,
   import_meta: Option<ast::VarDecl>,
@@ -828,7 +831,7 @@ impl<'a> Fold for DependencyCollector<'a> {
     };
 
     if is_require {
-      return ast::Expr::Ident(ast::Ident::new("undefined".into(), DUMMY_SP));
+      return ast::Expr::Ident(get_undefined_ident(self.unresolved_mark));
     }
 
     node.fold_children_with(self)
@@ -1096,7 +1099,7 @@ impl<'a> DependencyCollector<'a> {
   fn match_new_url(
     &mut self,
     expr: &ast::Expr,
-    decls: &HashSet<(JsWord, SyntaxContext)>,
+    decls: &HashSet<Id>,
   ) -> Option<(JsWord, swc_common::Span)> {
     use ast::*;
 
@@ -1139,6 +1142,7 @@ impl<'a> DependencyCollector<'a> {
     None
   }
 
+  #[allow(clippy::wrong_self_convention)]
   fn is_import_meta_url(&mut self, expr: &ast::Expr) -> bool {
     use ast::*;
 
@@ -1175,6 +1179,7 @@ impl<'a> DependencyCollector<'a> {
     }
   }
 
+  #[allow(clippy::wrong_self_convention)]
   fn is_import_meta(&mut self, expr: &ast::Expr) -> bool {
     use ast::*;
 

diff --git a/packages/transformers/js/core/src/env_replacer.rs b/packages/transformers/js/core/src/env_replacer.rs
@@ -2,7 +2,7 @@ use std::collections::{HashMap, HashSet};
 use std::vec;
 
 use swc_atoms::JsWord;
-use swc_common::{SyntaxContext, DUMMY_SP};
+use swc_common::{Mark, DUMMY_SP};
 use swc_ecmascript::ast;
 use swc_ecmascript::visit::{Fold, FoldWith};
 
@@ -13,10 +13,11 @@ pub struct EnvReplacer<'a> {
   pub replace_env: bool,
   pub is_browser: bool,
   pub env: &'a HashMap<swc_atoms::JsWord, swc_atoms::JsWord>,
-  pub decls: &'a HashSet<(JsWord, SyntaxContext)>,
+  pub decls: &'a HashSet<Id>,
   pub used_env: &'a mut HashSet<JsWord>,
   pub source_map: &'a swc_common::SourceMap,
   pub diagnostics: &'a mut Vec<Diagnostic>,
+  pub unresolved_mark: Mark,
 }
 
 impl<'a> Fold for EnvReplacer<'a> {
@@ -105,7 +106,7 @@ impl<'a> Fold for EnvReplacer<'a> {
                     right: Box::new(if let Some(init) = &decl.init {
                       *init.clone()
                     } else {
-                      Expr::Ident(Ident::new(js_word!("undefined"), DUMMY_SP))
+                      Expr::Ident(get_undefined_ident(self.unresolved_mark))
                     }),
                   }))
                 })
@@ -214,7 +215,7 @@ impl<'a> EnvReplacer<'a> {
         | "valueOf" => {}
         _ => {
           self.used_env.insert(sym.clone());
-          return Some(Expr::Ident(Ident::new(js_word!("undefined"), DUMMY_SP)));
+          return Some(Expr::Ident(get_undefined_ident(self.unresolved_mark)));
         }
       };
     }

diff --git a/packages/transformers/js/core/src/fs.rs b/packages/transformers/js/core/src/fs.rs
@@ -1,7 +1,7 @@
 use crate::dependency_collector::{DependencyDescriptor, DependencyKind};
 use crate::hoist::{Collect, Import};
 use crate::id;
-use crate::utils::{IdentId, SourceLocation};
+use crate::utils::SourceLocation;
 use data_encoding::{BASE64, HEXLOWER};
 use std::collections::HashSet;
 use std::path::{Path, PathBuf};
@@ -13,7 +13,7 @@ use swc_ecmascript::visit::{Fold, FoldWith, VisitWith};
 pub fn inline_fs<'a>(
   filename: &str,
   source_map: swc_common::sync::Lrc<swc_common::SourceMap>,
-  decls: HashSet<IdentId>,
+  decls: HashSet<Id>,
   global_mark: Mark,
   project_root: &'a str,
   deps: &'a mut Vec<DependencyDescriptor>,

diff --git a/packages/transformers/js/core/src/global_replacer.rs b/packages/transformers/js/core/src/global_replacer.rs
@@ -4,7 +4,7 @@ use std::path::Path;
 
 use swc_atoms::JsWord;
 use swc_common::{Mark, SourceMap, SyntaxContext, DUMMY_SP};
-use swc_ecmascript::ast::{self, ComputedPropName};
+use swc_ecmascript::ast::{self, ComputedPropName, Id};
 use swc_ecmascript::visit::{Fold, FoldWith};
 
 use crate::dependency_collector::{DependencyDescriptor, DependencyKind};
@@ -17,7 +17,7 @@ pub struct GlobalReplacer<'a> {
   pub globals: HashMap<JsWord, (SyntaxContext, ast::Stmt)>,
   pub project_root: &'a Path,
   pub filename: &'a Path,
-  pub decls: &'a mut HashSet<(JsWord, SyntaxContext)>,
+  pub decls: &'a mut HashSet<Id>,
   pub scope_hoist: bool,
 }
 

diff --git a/packages/transformers/js/core/src/hoist.rs b/packages/transformers/js/core/src/hoist.rs
@@ -1,4 +1,6 @@
-use crate::utils::{match_export_name, match_export_name_ident, match_property_name};
+use crate::utils::{
+  get_undefined_ident, match_export_name, match_export_name_ident, match_property_name,
+};
 use serde::{Deserialize, Serialize};
 use std::collections::hash_map::DefaultHasher;
 use std::collections::{HashMap, HashSet};
@@ -11,7 +13,7 @@ use swc_ecmascript::visit::{Fold, FoldWith, Visit, VisitWith};
 use crate::id;
 use crate::utils::{
   match_import, match_member_expr, match_require, Bailout, BailoutReason, CodeHighlight,
-  Diagnostic, DiagnosticSeverity, IdentId, SourceLocation,
+  Diagnostic, DiagnosticSeverity, SourceLocation,
 };
 
 macro_rules! hash {
@@ -25,9 +27,10 @@ macro_rules! hash {
 pub fn hoist(
   module: Module,
   module_id: &str,
+  unresolved_mark: Mark,
   collect: &Collect,
 ) -> Result<(Module, HoistResult, Vec<Diagnostic>), Vec<Diagnostic>> {
-  let mut hoist = Hoist::new(module_id, collect);
+  let mut hoist = Hoist::new(module_id, unresolved_mark, collect);
   let module = module.fold_with(&mut hoist);
 
   if !hoist.diagnostics.is_empty() {
@@ -66,6 +69,7 @@ struct Hoist<'a> {
   dynamic_imports: HashMap<JsWord, JsWord>,
   in_function_scope: bool,
   diagnostics: Vec<Diagnostic>,
+  unresolved_mark: Mark,
 }
 
 #[derive(Debug, Default, Serialize, Deserialize)]
@@ -83,7 +87,7 @@ pub struct HoistResult {
 }
 
 impl<'a> Hoist<'a> {
-  fn new(module_id: &'a str, collect: &'a Collect) -> Self {
+  fn new(module_id: &'a str, unresolved_mark: Mark, collect: &'a Collect) -> Self {
     Hoist {
       module_id,
       collect,
@@ -97,6 +101,7 @@ impl<'a> Hoist<'a> {
       dynamic_imports: HashMap::new(),
       in_function_scope: false,
       diagnostics: vec![],
+      unresolved_mark,
     }
   }
 
@@ -672,7 +677,7 @@ impl<'a> Fold for Hoist<'a> {
         if !self.in_function_scope {
           // If ESM, replace `this` with `undefined`, otherwise with the CJS exports object.
           if self.collect.is_esm {
-            return Expr::Ident(Ident::new("undefined".into(), DUMMY_SP));
+            return Expr::Ident(get_undefined_ident(self.unresolved_mark));
           } else if !self.collect.should_wrap {
             self.self_references.insert("*".into());
             return Expr::Ident(self.get_export_ident(this.span, &"*".into()));
@@ -1097,22 +1102,22 @@ pub struct Export {
 
 pub struct Collect {
   pub source_map: Lrc<swc_common::SourceMap>,
-  pub decls: HashSet<IdentId>,
+  pub decls: HashSet<Id>,
   pub ignore_mark: Mark,
   pub global_mark: Mark,
   pub static_cjs_exports: bool,
   pub has_cjs_exports: bool,
   pub is_esm: bool,
   pub should_wrap: bool,
   // local name -> descriptor
-  pub imports: HashMap<IdentId, Import>,
+  pub imports: HashMap<Id, Import>,
   // exported name -> descriptor
   pub exports: HashMap<JsWord, Export>,
   // local name -> exported name
-  pub exports_locals: HashMap<IdentId, JsWord>,
+  pub exports_locals: HashMap<Id, JsWord>,
   pub exports_all: HashMap<JsWord, SourceLocation>,
-  pub non_static_access: HashMap<IdentId, Vec<Span>>,
-  pub non_const_bindings: HashMap<IdentId, Vec<Span>>,
+  pub non_static_access: HashMap<Id, Vec<Span>>,
+  pub non_const_bindings: HashMap<Id, Vec<Span>>,
   pub non_static_requires: HashSet<JsWord>,
   pub wrapped_requires: HashSet<JsWord>,
   pub bailouts: Option<Vec<Bailout>>,
@@ -1156,7 +1161,7 @@ pub struct CollectResult {
 impl Collect {
   pub fn new(
     source_map: Lrc<swc_common::SourceMap>,
-    decls: HashSet<IdentId>,
+    decls: HashSet<Id>,
     ignore_mark: Mark,
     global_mark: Mark,
     trace_bailouts: bool,
@@ -2067,7 +2072,7 @@ impl Collect {
   }
 }
 
-fn has_binding_identifier(node: &Pat, sym: &JsWord, decls: &HashSet<IdentId>) -> bool {
+fn has_binding_identifier(node: &Pat, sym: &JsWord, decls: &HashSet<Id>) -> bool {
   match node {
     Pat::Ident(ident) => {
       if ident.id.sym == *sym && !decls.contains(&id!(ident.id)) {
@@ -2155,7 +2160,7 @@ mod tests {
             module.visit_with(&mut collect);
 
             let (module, res) = {
-              let mut hoist = Hoist::new("abc", &collect);
+              let mut hoist = Hoist::new("abc", unresolved_mark, &collect);
               let module = module.fold_with(&mut hoist);
               (module, hoist.get_result())
             };