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

Respect webpackIgnore and turbopackIgnore comments in the parser #68451

Merged
merged 1 commit into from
Aug 27, 2024
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion turbopack/crates/turbopack-ecmascript/benches/analyzer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ pub fn benchmark(c: &mut Criterion) {
program.visit_mut_with(&mut resolver(unresolved_mark, top_level_mark, false));

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

let input = BenchInput {
Expand Down
14 changes: 14 additions & 0 deletions turbopack/crates/turbopack-ecmascript/readme.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# turbopack-ecmascript

## Adding new parser tests

We use a snapshot-based testing system to ensure that changes to the parser don't break existing code.
To add a new test, you need to create a new directory in `tests/analyzer/graph` with an 'input.js' file
inside.

The snapshot tests are done with the `testing` crate. You can upate them by passing the env var
`UPDATE=1` to the test runner.

```sh
UPDATE=1 cargo test -p turbopack-ecmascript
```
10 changes: 8 additions & 2 deletions turbopack/crates/turbopack-ecmascript/src/analyzer/graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use std::{

use swc_core::{
atoms::Atom,
common::{pass::AstNodePath, Mark, Span, Spanned, SyntaxContext, GLOBALS},
common::{comments::Comments, pass::AstNodePath, Mark, Span, Spanned, SyntaxContext, GLOBALS},
ecma::{
ast::*,
atoms::js_word,
Expand Down Expand Up @@ -294,23 +294,29 @@ pub fn create_graph(m: &Program, eval_context: &EvalContext) -> VarGraph {
graph
}

/// A context used for assembling the evaluation graph.
#[derive(Debug)]
pub struct EvalContext {
pub(crate) unresolved_mark: Mark,
pub(crate) top_level_mark: Mark,
pub(crate) imports: ImportMap,
}

impl EvalContext {
/// Produce a new [EvalContext] from a [Program]. If you wish to support
/// webpackIgnore or turbopackIgnore comments, you must pass those in,
/// since the AST does not include comments by default.
pub fn new(
module: &Program,
unresolved_mark: Mark,
top_level_mark: Mark,
comments: Option<&dyn Comments>,
source: Option<Vc<Box<dyn Source>>>,
) -> Self {
Self {
unresolved_mark,
top_level_mark,
imports: ImportMap::analyze(module, source),
imports: ImportMap::analyze(module, source, comments),
}
}

Expand Down
85 changes: 80 additions & 5 deletions turbopack/crates/turbopack-ecmascript/src/analyzer/imports.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
use std::{collections::BTreeMap, fmt::Display};
use std::{
collections::{BTreeMap, HashMap},
fmt::Display,
};

use indexmap::{IndexMap, IndexSet};
use once_cell::sync::Lazy;
use swc_core::{
common::{source_map::SmallPos, Span},
common::{comments::Comments, source_map::SmallPos, Span, Spanned},
ecma::{
ast::*,
atoms::{js_word, JsWord},
Expand Down Expand Up @@ -136,6 +139,17 @@ pub(crate) struct ImportMap {

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

/// Locations of webpackIgnore or turbopackIgnore comments
/// This is a webpack feature that allows opting out of static
/// imports, which we should respect.
///
/// Example:
/// ```js
/// const a = import(/* webpackIgnore: true */ "a");
/// const b = import(/* turbopackIgnore: true */ "b");
/// ```
turbopack_ignores: HashMap<Span, bool>,
}

#[derive(Debug, Clone, PartialEq, Eq, Hash)]
Expand Down Expand Up @@ -200,12 +214,17 @@ impl ImportMap {
}

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

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

data
Expand All @@ -215,6 +234,7 @@ impl ImportMap {
struct Analyzer<'a> {
data: &'a mut ImportMap,
source: Option<Vc<Box<dyn Source>>>,
comments: Option<&'a dyn Comments>,
}

impl<'a> Analyzer<'a> {
Expand Down Expand Up @@ -409,8 +429,63 @@ impl Visit for Analyzer<'_> {
fn visit_export_default_expr(&mut self, _: &ExportDefaultExpr) {
self.data.has_exports = true;
}
fn visit_stmt(&mut self, _: &Stmt) {
// don't visit children

fn visit_stmt(&mut self, n: &Stmt) {
if self.comments.is_some() {
// only visit children if we potentially need to mark import / requires
n.visit_children_with(self);
}
}

arlyon marked this conversation as resolved.
Show resolved Hide resolved
/// check if import or require contains an ignore comment
///
/// We are checking for the following cases:
/// - import(/* webpackIgnore: true */ "a")
/// - require(/* webpackIgnore: true */ "a")
///
/// We can do this by checking if any of the comment spans are between the
/// callee and the first argument.
fn visit_call_expr(&mut self, n: &CallExpr) {
// we can actually unwrap thanks to the optimisation above
// but it can't hurt to be safe...
if let Some(comments) = self.comments {
let callee_span = match &n.callee {
Callee::Import(Import { span, .. }) => Some(span),
// this assumes you cannot reassign `require`
Callee::Expr(box Expr::Ident(Ident { span, sym, .. })) if sym == "require" => {
Some(span)
}
_ => None,
};

// we are interested here in the last comment with a valid directive
let ignore_statement = n
.args
.first()
.map(|arg| arg.span_lo())
.and_then(|comment_pos| comments.get_leading(comment_pos))
.iter()
.flatten()
.rev()
.filter_map(|comment| {
let (directive, value) = comment.text.trim().split_once(':')?;
// support whitespace between the colon
match (directive.trim(), value.trim()) {
("webpackIgnore" | "turbopackIgnore", "true") => Some(true),
("webpackIgnore" | "turbopackIgnore", "false") => Some(false),
_ => None, // ignore anything else
}
})
.next();

if let Some((callee_span, ignore_statement)) = callee_span.zip(ignore_statement) {
self.data
.turbopack_ignores
.insert(*callee_span, ignore_statement);
};
}

n.visit_children_with(self);
}

fn visit_program(&mut self, m: &Program) {
Expand Down
3 changes: 2 additions & 1 deletion turbopack/crates/turbopack-ecmascript/src/analyzer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3822,7 +3822,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, top_level_mark, None);
let eval_context =
EvalContext::new(&m, unresolved_mark, top_level_mark, None, None);

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

Expand Down
2 changes: 1 addition & 1 deletion turbopack/crates/turbopack-ecmascript/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,7 @@ impl EcmascriptParsable for EcmascriptModuleAsset {
impl EcmascriptAnalyzable for EcmascriptModuleAsset {
#[turbo_tasks::function]
fn analyze(self: Vc<Self>) -> Vc<AnalyzeEcmascriptModuleResult> {
analyse_ecmascript_module(self, None)
analyse_ecmascript_module(self, None, None)
}

/// Generates module contents without an analysis pass. This is useful for
Expand Down
1 change: 1 addition & 0 deletions turbopack/crates/turbopack-ecmascript/src/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -433,6 +433,7 @@ async fn parse_content(
&parsed_program,
unresolved_mark,
top_level_mark,
None,
Some(source),
);

Expand Down
12 changes: 10 additions & 2 deletions turbopack/crates/turbopack-ecmascript/src/references/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ pub mod util;

use std::{
borrow::Cow,
collections::{BTreeMap, HashMap},
collections::{BTreeMap, HashMap, HashSet},
future::Future,
mem::take,
pin::Pin,
Expand Down Expand Up @@ -393,16 +393,23 @@ where
HANDLER.set(handler, || GLOBALS.set(globals, f))
}

/// Analyse a provided [EcmascriptModuleAsset] and return a
/// [AnalyzeEcmascriptModuleResult].
///
/// # Arguments
/// * ignored_spans - A set of spans to ignore when analysing the module. This is useful for example
/// to respect turbopackIgnore directives on ignores.
#[turbo_tasks::function]
pub(crate) async fn analyse_ecmascript_module(
module: Vc<EcmascriptModuleAsset>,
part: Option<Vc<ModulePart>>,
ignored_spans: Option<Vc<HashSet<Span>>>,
) -> Result<Vc<AnalyzeEcmascriptModuleResult>> {
let span = {
let module = module.ident().to_string().await?.to_string();
tracing::info_span!("analyse ecmascript module", module = module)
};
let result = analyse_ecmascript_module_internal(module, part)
let result = analyse_ecmascript_module_internal(module, part, ignored_spans)
.instrument(span)
.await;

Expand All @@ -418,6 +425,7 @@ pub(crate) async fn analyse_ecmascript_module(
pub(crate) async fn analyse_ecmascript_module_internal(
module: Vc<EcmascriptModuleAsset>,
part: Option<Vc<ModulePart>>,
_webpack_ignored_effects: Option<Vc<HashSet<Span>>>,
) -> Result<Vc<AnalyzeEcmascriptModuleResult>> {
let raw_module = module.await?;

Expand Down
18 changes: 13 additions & 5 deletions turbopack/crates/turbopack-ecmascript/src/tree_shake/asset.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
use std::collections::HashSet;

use anyhow::{Context, Result};
use swc_core::common::Span;
use turbo_tasks::Vc;
use turbopack_core::{
asset::{Asset, AssetContent},
Expand Down Expand Up @@ -59,7 +62,11 @@ impl EcmascriptAnalyzable for EcmascriptModulePartAsset {
async fn analyze(self: Vc<Self>) -> Result<Vc<AnalyzeEcmascriptModuleResult>> {
let this = self.await?;
let part = this.part;
Ok(analyse_ecmascript_module(this.full_module, Some(part)))
Ok(analyse_ecmascript_module(
this.full_module,
Some(part),
None,
))
}

#[turbo_tasks::function]
Expand Down Expand Up @@ -131,7 +138,7 @@ impl Module for EcmascriptModulePartAsset {
async fn references(&self) -> Result<Vc<ModuleReferences>> {
let split_data = split_module(self.full_module).await?;

let analyze = analyze(self.full_module, self.part).await?;
let analyze = analyze(self.full_module, self.part, None).await?;

let (deps, entrypoints) = match &*split_data {
SplitResult::Ok {
Expand Down Expand Up @@ -264,16 +271,17 @@ impl EcmascriptModulePartAsset {
pub(super) async fn analyze(self: Vc<Self>) -> Result<Vc<AnalyzeEcmascriptModuleResult>> {
let this = self.await?;

Ok(analyze(this.full_module, this.part))
Ok(analyze(this.full_module, this.part, None))
}
}

#[turbo_tasks::function]
async fn analyze(
fn analyze(
module: Vc<EcmascriptModuleAsset>,
part: Vc<ModulePart>,
ignored_spans: Option<Vc<HashSet<Span>>>,
) -> Result<Vc<AnalyzeEcmascriptModuleResult>> {
Ok(analyse_ecmascript_module(module, Some(part)))
Ok(analyse_ecmascript_module(module, Some(part), ignored_spans))
}

#[turbo_tasks::value_impl]
Expand Down
3 changes: 3 additions & 0 deletions turbopack/crates/turbopack-ecmascript/src/tree_shake/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -505,6 +505,7 @@ pub(super) async fn split(
&program,
eval_context.unresolved_mark,
eval_context.top_level_mark,
None,
Some(source),
);

Expand Down Expand Up @@ -629,6 +630,7 @@ pub(super) async fn part_of_module(
eval_context.unresolved_mark,
eval_context.top_level_mark,
None,
None,
);

return Ok(ParseResult::Ok {
Expand Down Expand Up @@ -706,6 +708,7 @@ pub(super) async fn part_of_module(
eval_context.unresolved_mark,
eval_context.top_level_mark,
None,
None,
);
return Ok(ParseResult::Ok {
program,
Expand Down
Loading
Loading