Skip to content

Commit

Permalink
fix(turbopack): Fix edge cases of tree shaking (vercel/turborepo#7986)
Browse files Browse the repository at this point in the history
### Description

Closes PACK-2966

### Testing Instructions

I'll add some tests

---------

Co-authored-by: Tobias Koppers <tobias.koppers@googlemail.com>
  • Loading branch information
kdy1 and sokra committed May 28, 2024
1 parent 0887f31 commit faeb2ef
Show file tree
Hide file tree
Showing 208 changed files with 13,170 additions and 1,271 deletions.
1 change: 1 addition & 0 deletions crates/turbo-tasks-memory/src/aggregation/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ fn check_invariants<'a>(
false
}
});
#[allow(clippy::never_loop)]
for missing_upper in missing_uppers {
let upper_value = {
let upper = ctx.node(missing_upper);
Expand Down
4 changes: 3 additions & 1 deletion crates/turbopack-core/src/context.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use anyhow::{bail, Result};
use turbo_tasks::{Value, Vc};
use turbo_tasks_fs::FileSystemPath;
use turbo_tasks_fs::{glob::Glob, FileSystemPath};

use crate::{
compile_time_info::CompileTimeInfo,
Expand Down Expand Up @@ -76,4 +76,6 @@ pub trait AssetContext {

/// Gets a new AssetContext with the transition applied.
fn with_transition(self: Vc<Self>, transition: String) -> Vc<Box<dyn AssetContext>>;

fn side_effect_free_packages(self: Vc<Self>) -> Vc<Glob>;
}
11 changes: 11 additions & 0 deletions crates/turbopack-core/src/resolve/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,17 @@ impl ModuleResolveResult {
}
}

pub fn ignored() -> ModuleResolveResult {
Self::ignored_with_key(RequestKey::default())
}

pub fn ignored_with_key(request_key: RequestKey) -> ModuleResolveResult {
ModuleResolveResult {
primary: indexmap! { request_key => ModuleResolveResultItem::Ignore },
affecting_sources: Vec::new(),
}
}

pub fn module(module: Vc<Box<dyn Module>>) -> ModuleResolveResult {
Self::module_with_key(RequestKey::default(), module)
}
Expand Down
17 changes: 15 additions & 2 deletions crates/turbopack-ecmascript-runtime/js/src/shared/runtime-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,25 @@ function defineProp(
/**
* Adds the getters to the exports object.
*/
function esm(exports: Exports, getters: Record<string, () => any>) {
function esm(
exports: Exports,
getters: Record<string, (() => any) | [() => any, (v: any) => void]>
) {
defineProp(exports, "__esModule", { value: true });
if (toStringTag) defineProp(exports, toStringTag, { value: "Module" });
for (const key in getters) {
defineProp(exports, key, { get: getters[key], enumerable: true });
const item = getters[key];
if (Array.isArray(item)) {
defineProp(exports, key, {
get: item[0],
set: item[1],
enumerable: true,
});
} else {
defineProp(exports, key, { get: item, enumerable: true });
}
}
Object.seal(exports);
}

/**
Expand Down
3 changes: 2 additions & 1 deletion crates/turbopack-ecmascript/benches/analyzer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ pub fn benchmark(c: &mut Criterion) {
let top_level_mark = Mark::new();
program.visit_mut_with(&mut resolver(unresolved_mark, top_level_mark, false));

let eval_context = EvalContext::new(&program, unresolved_mark, None);
let eval_context =
EvalContext::new(&program, unresolved_mark, top_level_mark, false, None);
let var_graph = create_graph(&program, &eval_context);

let input = BenchInput {
Expand Down
19 changes: 16 additions & 3 deletions crates/turbopack-ecmascript/src/analyzer/graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -261,18 +261,22 @@ pub fn create_graph(m: &Program, eval_context: &EvalContext) -> VarGraph {

pub struct EvalContext {
pub(crate) unresolved_mark: Mark,
pub(crate) top_level_mark: Mark,
pub(crate) imports: ImportMap,
}

impl EvalContext {
pub fn new(
module: &Program,
unresolved_mark: Mark,
top_level_mark: Mark,
skip_namespace: bool,
source: Option<Vc<Box<dyn Source>>>,
) -> Self {
Self {
unresolved_mark,
imports: ImportMap::analyze(module, source),
top_level_mark,
imports: ImportMap::analyze(module, skip_namespace, source),
}
}

Expand Down Expand Up @@ -1460,6 +1464,8 @@ impl VisitAstPath for Analyzer<'_> {
) {
let value = self.current_value.take();
if let SimpleAssignTarget::Ident(i) = n {
n.visit_children_with_path(self, ast_path);

self.add_value(
i.to_id(),
value.unwrap_or_else(|| {
Expand Down Expand Up @@ -1572,13 +1578,20 @@ impl VisitAstPath for Analyzer<'_> {
ident: &'ast Ident,
ast_path: &mut AstNodePath<AstParentNodeRef<'r>>,
) {
if !matches!(
if !(matches!(
ast_path.last(),
Some(AstParentNodeRef::Expr(_, ExprField::Ident))
| Some(AstParentNodeRef::Prop(_, PropField::Shorthand))
) {
) || matches!(
ast_path.get(ast_path.len() - 2),
Some(AstParentNodeRef::SimpleAssignTarget(
_,
SimpleAssignTargetField::Ident,
))
)) {
return;
}

if let Some((esm_reference_index, export)) =
self.eval_context.imports.get_binding(&ident.to_id())
{
Expand Down
89 changes: 75 additions & 14 deletions crates/turbopack-ecmascript/src/analyzer/imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ use swc_core::{
use turbo_tasks::Vc;
use turbopack_core::{issue::IssueSource, source::Source};

use super::{JsValue, ModuleValue};
use super::{top_level_await::has_top_level_await, JsValue, ModuleValue};
use crate::tree_shake::{find_turbopack_part_id_in_asserts, PartId};

#[turbo_tasks::value(serialization = "auto_for_input")]
#[derive(Default, Debug, Clone, Hash, PartialOrd, Ord)]
Expand Down Expand Up @@ -129,16 +130,22 @@ pub(crate) struct ImportMap {

/// True, when the module has exports
has_exports: bool,

/// True if the module is an ESM module due to top-level await.
has_top_level_await: bool,
}

#[derive(Debug, PartialEq, Eq, Hash, PartialOrd, Ord)]
#[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
pub(crate) enum ImportedSymbol {
ModuleEvaluation,
Symbol(JsWord),
/// User requested the whole module
Namespace,
Exports,
Part(u32),
}

#[derive(Debug, PartialEq, Eq, Hash, PartialOrd, Ord)]
#[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
pub(crate) struct ImportMapReference {
pub module_path: JsWord,
pub imported_symbol: ImportedSymbol,
Expand All @@ -148,7 +155,10 @@ pub(crate) struct ImportMapReference {

impl ImportMap {
pub fn is_esm(&self) -> bool {
self.has_exports || !self.imports.is_empty() || !self.namespace_imports.is_empty()
self.has_exports
|| self.has_top_level_await
|| !self.imports.is_empty()
|| !self.namespace_imports.is_empty()
}

pub fn get_import(&self, id: &Id) -> Option<JsValue> {
Expand Down Expand Up @@ -192,11 +202,16 @@ impl ImportMap {
}

/// Analyze ES import
pub(super) fn analyze(m: &Program, source: Option<Vc<Box<dyn Source>>>) -> Self {
pub(super) fn analyze(
m: &Program,
skip_namespace: bool,
source: Option<Vc<Box<dyn Source>>>,
) -> Self {
let mut data = ImportMap::default();

m.visit_with(&mut Analyzer {
data: &mut data,
skip_namespace,
source,
});

Expand All @@ -206,6 +221,7 @@ impl ImportMap {

struct Analyzer<'a> {
data: &'a mut ImportMap,
skip_namespace: bool,
source: Option<Vc<Box<dyn Source>>>,
}

Expand All @@ -216,7 +232,11 @@ impl<'a> Analyzer<'a> {
module_path: JsWord,
imported_symbol: ImportedSymbol,
annotations: ImportAnnotations,
) -> usize {
) -> Option<usize> {
if self.skip_namespace && matches!(imported_symbol, ImportedSymbol::Namespace) {
return None;
}

let issue_source = self
.source
.map(|s| IssueSource::from_swc_offsets(s, span.lo.to_usize(), span.hi.to_usize()));
Expand All @@ -228,11 +248,11 @@ impl<'a> Analyzer<'a> {
annotations,
};
if let Some(i) = self.data.references.get_index_of(&r) {
i
Some(i)
} else {
let i = self.data.references.len();
self.data.references.insert(r);
i
Some(i)
}
}
}
Expand All @@ -256,13 +276,17 @@ impl Visit for Analyzer<'_> {
);

for s in &import.specifiers {
let symbol = get_import_symbol_from_import(s);
let symbol = get_import_symbol_from_import(s, import.with.as_deref());
let i = self.ensure_reference(
import.span,
import.src.value.clone(),
symbol,
annotations.clone(),
);
let i = match i {
Some(v) => v,
None => continue,
};

let (local, orig_sym) = match s {
ImportSpecifier::Named(ImportNamedSpecifier {
Expand Down Expand Up @@ -293,13 +317,17 @@ impl Visit for Analyzer<'_> {
ImportedSymbol::ModuleEvaluation,
annotations.clone(),
);
let symbol = parse_with(export.with.as_deref());

let i = self.ensure_reference(
export.span,
export.src.value.clone(),
ImportedSymbol::Namespace,
symbol.unwrap_or(ImportedSymbol::Namespace),
annotations,
);
self.data.reexports.push((i, Reexport::Star));
if let Some(i) = i {
self.data.reexports.push((i, Reexport::Star));
}
}

fn visit_named_export(&mut self, export: &NamedExport) {
Expand All @@ -319,10 +347,14 @@ impl Visit for Analyzer<'_> {
);

for spec in export.specifiers.iter() {
let symbol = get_import_symbol_from_export(spec);
let symbol = get_import_symbol_from_export(spec, export.with.as_deref());

let i =
self.ensure_reference(export.span, src.value.clone(), symbol, annotations.clone());
let i = match i {
Some(v) => v,
None => continue,
};

match spec {
ExportSpecifier::Namespace(n) => {
Expand Down Expand Up @@ -367,6 +399,12 @@ impl Visit for Analyzer<'_> {
fn visit_stmt(&mut self, _: &Stmt) {
// don't visit children
}

fn visit_program(&mut self, m: &Program) {
self.data.has_top_level_await = has_top_level_await(m).is_some();

m.visit_children_with(self);
}
}

pub(crate) fn orig_name(n: &ModuleExportName) -> JsWord {
Expand All @@ -376,7 +414,23 @@ pub(crate) fn orig_name(n: &ModuleExportName) -> JsWord {
}
}

fn get_import_symbol_from_import(specifier: &ImportSpecifier) -> ImportedSymbol {
fn parse_with(with: Option<&ObjectLit>) -> Option<ImportedSymbol> {
find_turbopack_part_id_in_asserts(with?).map(|v| match v {
PartId::Internal(index) => ImportedSymbol::Part(index),
PartId::ModuleEvaluation => ImportedSymbol::ModuleEvaluation,
PartId::Export(e) => ImportedSymbol::Symbol(e.into()),
PartId::Exports => ImportedSymbol::Exports,
})
}

fn get_import_symbol_from_import(
specifier: &ImportSpecifier,
with: Option<&ObjectLit>,
) -> ImportedSymbol {
if let Some(part) = parse_with(with) {
return part;
}

match specifier {
ImportSpecifier::Named(ImportNamedSpecifier {
local, imported, ..
Expand All @@ -389,7 +443,14 @@ fn get_import_symbol_from_import(specifier: &ImportSpecifier) -> ImportedSymbol
}
}

fn get_import_symbol_from_export(specifier: &ExportSpecifier) -> ImportedSymbol {
fn get_import_symbol_from_export(
specifier: &ExportSpecifier,
with: Option<&ObjectLit>,
) -> ImportedSymbol {
if let Some(part) = parse_with(with) {
return part;
}

match specifier {
ExportSpecifier::Named(ExportNamedSpecifier { orig, .. }) => {
ImportedSymbol::Symbol(orig_name(orig))
Expand Down
3 changes: 2 additions & 1 deletion crates/turbopack-ecmascript/src/analyzer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3574,7 +3574,8 @@ mod tests {
let top_level_mark = Mark::new();
m.visit_mut_with(&mut resolver(unresolved_mark, top_level_mark, false));

let eval_context = EvalContext::new(&m, unresolved_mark, None);
let eval_context =
EvalContext::new(&m, unresolved_mark, top_level_mark, false, None);

let mut var_graph = create_graph(&m, &eval_context);

Expand Down
3 changes: 1 addition & 2 deletions crates/turbopack-ecmascript/src/chunk/placeable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,7 @@ pub async fn is_marked_as_side_effect_free(
return Ok(Vc::cell(true));
}

let find_package_json: turbo_tasks::ReadRef<FindContextFileResult> =
find_context_file(path.parent(), package_json()).await?;
let find_package_json = find_context_file(path.parent(), package_json()).await?;

if let FindContextFileResult::Found(package_json, _) = *find_package_json {
match *side_effects_from_package_json(package_json).await? {
Expand Down
1 change: 1 addition & 0 deletions crates/turbopack-ecmascript/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,7 @@ struct MemoizedSuccessfulAnalysis {
source_map: Option<ReadRef<SourceMap>>,
}

#[derive(Clone)]
pub struct EcmascriptModuleAssetBuilder {
source: Vc<Box<dyn Source>>,
asset_context: Vc<Box<dyn AssetContext>>,
Expand Down
Loading

0 comments on commit faeb2ef

Please sign in to comment.