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.

Sign up for GitHub

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

Small tree shaking fix and test case update #8347

Merged
merged 7 commits into from
Jun 7, 2024
Merged

Small tree shaking fix and test case update #8347

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
dc8786e
add tree shaking broken test case
sokra Jun 6, 2024
052e1ac
avoid adding module evaluation to internal imports
sokra Jun 6, 2024
9eecab6
make test case more clear
sokra Jun 6, 2024
aed3c5e
Named export should not create an item
kdy1 Jun 7, 2024
28409e6
Update test refs
kdy1 Jun 7, 2024
d44a24a
Fix named export
kdy1 Jun 7, 2024
f49dfe4
Update test refs
kdy1 Jun 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 28 additions & 30 deletions crates/turbopack-ecmascript/src/analyzer/imports.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,15 +268,21 @@ impl Visit for Analyzer<'_> {
fn visit_import_decl(&mut self, import: &ImportDecl) {
let annotations = ImportAnnotations::parse(import.with.as_deref());

self.ensure_reference(
import.span,
import.src.value.clone(),
ImportedSymbol::ModuleEvaluation,
annotations.clone(),
);
let internal_symbol = parse_with(import.with.as_deref());

if internal_symbol.is_none() || import.specifiers.is_empty() {
self.ensure_reference(
import.span,
import.src.value.clone(),
ImportedSymbol::ModuleEvaluation,
annotations.clone(),
);
}

for s in &import.specifiers {
let symbol = get_import_symbol_from_import(s, import.with.as_deref());
let symbol = internal_symbol
.clone()
.unwrap_or_else(|| get_import_symbol_from_import(s));
let i = self.ensure_reference(
import.span,
import.src.value.clone(),
Expand Down Expand Up @@ -339,15 +345,21 @@ impl Visit for Analyzer<'_> {

let annotations = ImportAnnotations::parse(export.with.as_deref());

self.ensure_reference(
export.span,
src.value.clone(),
ImportedSymbol::ModuleEvaluation,
annotations.clone(),
);
let internal_symbol = parse_with(export.with.as_deref());

if internal_symbol.is_none() || export.specifiers.is_empty() {
self.ensure_reference(
export.span,
src.value.clone(),
ImportedSymbol::ModuleEvaluation,
annotations.clone(),
);
}

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

let i =
self.ensure_reference(export.span, src.value.clone(), symbol, annotations.clone());
Expand Down Expand Up @@ -423,14 +435,7 @@ fn parse_with(with: Option<&ObjectLit>) -> Option<ImportedSymbol> {
})
}

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

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

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

fn get_import_symbol_from_export(specifier: &ExportSpecifier) -> ImportedSymbol {
match specifier {
ExportSpecifier::Named(ExportNamedSpecifier { orig, .. }) => {
ImportedSymbol::Symbol(orig_name(orig))
Expand Down
14 changes: 0 additions & 14 deletions crates/turbopack-ecmascript/src/references/esm/base.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ use turbopack_core::{
ChunkItemExt, ChunkableModule, ChunkableModuleReference, ChunkingContext, ChunkingType,
ChunkingTypeOption, ModuleId,
},
context::AssetContext,
issue::{IssueSeverity, IssueSource},
module::Module,
reference::ModuleReference,
Expand Down Expand Up @@ -152,7 +151,6 @@ impl ModuleReference for EsmAssetReference {
EcmaScriptModulesReferenceSubType::Import
};

// Skip side effect free self-references here.
if let Request::Module { module, .. } = &*self.request.await? {
if module == TURBOPACK_PART_IMPORT_SOURCE {
if let Some(part) = self.export_name {
Expand All @@ -161,18 +159,6 @@ impl ModuleReference for EsmAssetReference {
.await?
.expect("EsmAssetReference origin should be a EcmascriptModuleAsset");

let side_effect_free_packages =
full_module.asset_context().side_effect_free_packages();

if let ModulePart::Evaluation = *part.await? {
if *full_module
.is_marked_as_side_effect_free(side_effect_free_packages)
.await?
{
return Ok(ModuleResolveResult::ignored().cell());
}
}

let module =
EcmascriptModulePartAsset::new(full_module, part, self.import_externals);

Expand Down
4 changes: 2 additions & 2 deletions crates/turbopack-ecmascript/src/tree_shake/graph.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -607,7 +607,7 @@ impl DepGraph {
let (orig, mut local, exported) = match s {
ExportSpecifier::Named(s) => (
Some(s.orig.clone()),
match s.exported.as_ref().unwrap_or(&s.orig) {
match &s.orig {
ModuleExportName::Ident(i) => i.clone(),
ModuleExportName::Str(..) => quote_ident!("_tmp"),
},
Expand Down Expand Up @@ -1006,7 +1006,7 @@ impl DepGraph {
ModuleItem::ModuleDecl(
ModuleDecl::ExportDefaultDecl(..)
| ModuleDecl::ExportDefaultExpr(..)
| ModuleDecl::ExportNamed(NamedExport { src: Some(..), .. }),
| ModuleDecl::ExportNamed(NamedExport { .. }),
) => {}

_ => {
Expand Down
Loading
Loading