Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Apply unresolved mark to inserted undefined identifiers #8151

Merged
merged 4 commits into from May 27, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -0,0 +1,7 @@
module.exports = function () {
if(process.env.ABC === 'a') {
return require("./unused.js");
} else {
return "ok";
}
};
@@ -0,0 +1 @@
module.exports = "unused";
22 changes: 22 additions & 0 deletions packages/core/integration-tests/test/javascript.js
Expand Up @@ -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(
Expand Down
8 changes: 3 additions & 5 deletions 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,
Expand All @@ -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,
}

Expand Down
17 changes: 11 additions & 6 deletions packages/transformers/js/core/src/dependency_collector.rs
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand All @@ -72,6 +73,7 @@ pub fn dependency_collector<'a>(
require_node: None,
decls,
ignore_mark,
unresolved_mark,
config,
diagnostics,
import_meta: None,
Expand All @@ -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>,
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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::*;

Expand Down Expand Up @@ -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::*;

Expand Down Expand Up @@ -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::*;

Expand Down
9 changes: 5 additions & 4 deletions packages/transformers/js/core/src/env_replacer.rs
Expand Up @@ -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};

Expand All @@ -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> {
Expand Down Expand Up @@ -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))
}),
}))
})
Expand Down Expand Up @@ -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)));
}
};
}
Expand Down
4 changes: 2 additions & 2 deletions 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};
Expand All @@ -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>,
Expand Down
4 changes: 2 additions & 2 deletions packages/transformers/js/core/src/global_replacer.rs
Expand Up @@ -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};
Expand All @@ -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,
}

Expand Down
31 changes: 18 additions & 13 deletions 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};
Expand All @@ -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 {
Expand All @@ -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() {
Expand Down Expand Up @@ -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)]
Expand All @@ -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,
Expand All @@ -97,6 +101,7 @@ impl<'a> Hoist<'a> {
dynamic_imports: HashMap::new(),
in_function_scope: false,
diagnostics: vec![],
unresolved_mark,
}
}

Expand Down Expand Up @@ -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()));
Expand Down Expand Up @@ -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>>,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -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())
};
Expand Down