Skip to content

Commit 9f80374

Browse files
mischnicmischnic
committed
Turbopack: correctly trace files with npm
1 parent 3cf4b17 commit 9f80374

File tree

3 files changed

+101
-99
lines changed

3 files changed

+101
-99
lines changed

‎crates/next-api/src/module_graph.rs‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -568,14 +568,7 @@ async fn validate_pages_css_imports(
568568
.map(async |issue| {
569569
// We allow imports of global CSS files which are inside of `node_modules`.
570570
Ok(
571-
if !issue
572-
.module
573-
.ident()
574-
.path()
575-
.await?
576-
.path
577-
.contains("/node_modules/")
578-
{
571+
if !issue.module.ident().path().await?.is_in_node_modules() {
579572
Some(issue)
580573
} else {
581574
None

‎turbopack/crates/turbo-tasks-fs/src/lib.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1239,6 +1239,10 @@ impl FileSystemPath {
12391239
self.path.is_empty()
12401240
}
12411241

1242+
pub fn is_in_node_modules(&self) -> bool {
1243+
self.path.starts_with("node_modules/") || self.path.contains("/node_modules/")
1244+
}
1245+
12421246
/// Returns the path of `inner` relative to `self`.
12431247
///
12441248
/// Note: this method always strips the leading `/` from the result.

‎turbopack/crates/turbopack-ecmascript/src/references/mod.rs‎

Lines changed: 96 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -974,12 +974,7 @@ async fn analyze_ecmascript_module_internal(
974974
origin,
975975
compile_time_info,
976976
var_graph: &var_graph,
977-
allow_project_root_tracing: !source
978-
.ident()
979-
.path()
980-
.await?
981-
.path
982-
.contains("/node_modules/"),
977+
allow_project_root_tracing: !source.ident().path().await?.is_in_node_modules(),
983978
fun_args_values: Default::default(),
984979
var_cache: Default::default(),
985980
first_import_meta: true,
@@ -1742,15 +1737,21 @@ async fn handle_call<G: Fn(Vec<Effect>) + Send + Sync>(
17421737
return Ok(());
17431738
}
17441739

1745-
let get_traced_project_dir = async || {
1740+
let get_traced_project_dir = async || -> Result<FileSystemPath> {
17461741
// readFileSync("./foo") should always be relative to the project root, but this is
1747-
// dangerous inside of node_modules as it can cause a lot of false positives in the
1748-
// tracing, if some package does `path.join(dynamic)`, it would include everything from
1749-
// the project root as well.
1750-
if allow_project_root_tracing {
1751-
compile_time_info.environment().cwd().owned().await
1742+
// dangerous inside of node_modules as it can cause a lot of false positives in the tracing,
1743+
// if some package does `path.join(dynamic)`, it would include everything from the project
1744+
// root as well.
1745+
//
1746+
// Also, when there's no cwd set (i.e. in a tracing-specific module context, as we shouldn't
1747+
// assume a `process.cwd()` for all of node_modules), fallback to the source file directory.
1748+
// This still allows relative file accesses, just not from the project root.
1749+
if allow_project_root_tracing
1750+
&& let Some(cwd) = compile_time_info.environment().cwd().owned().await?
1751+
{
1752+
Ok(cwd)
17521753
} else {
1753-
Ok(Some(source.ident().path().await?.parent()))
1754+
Ok(source.ident().path().await?.parent())
17541755
}
17551756
};
17561757

@@ -1982,18 +1983,16 @@ async fn handle_call<G: Fn(Vec<Effect>) + Send + Sync>(
19821983
return Ok(());
19831984
}
19841985
}
1985-
if let Some(context_dir) = get_traced_project_dir().await? {
1986-
analysis.add_reference(
1987-
FileSourceReference::new(
1988-
context_dir,
1989-
Pattern::new(pat),
1990-
collect_affecting_sources,
1991-
make_issue_source(),
1992-
)
1993-
.to_resolved()
1994-
.await?,
1995-
);
1996-
}
1986+
analysis.add_reference(
1987+
FileSourceReference::new(
1988+
get_traced_project_dir().await?,
1989+
Pattern::new(pat),
1990+
collect_affecting_sources,
1991+
make_issue_source(),
1992+
)
1993+
.to_resolved()
1994+
.await?,
1995+
);
19971996
return Ok(());
19981997
}
19991998
let (args, hints) = explain_args(&args);
@@ -2038,13 +2037,15 @@ async fn handle_call<G: Fn(Vec<Effect>) + Send + Sync>(
20382037
return Ok(());
20392038
}
20402039
}
2041-
if let Some(context_dir) = get_traced_project_dir().await? {
2042-
analysis.add_reference(
2043-
DirAssetReference::new(context_dir, Pattern::new(pat), make_issue_source())
2044-
.to_resolved()
2045-
.await?,
2046-
);
2047-
}
2040+
analysis.add_reference(
2041+
DirAssetReference::new(
2042+
get_traced_project_dir().await?,
2043+
Pattern::new(pat),
2044+
make_issue_source(),
2045+
)
2046+
.to_resolved()
2047+
.await?,
2048+
);
20482049
return Ok(());
20492050
}
20502051

@@ -2067,6 +2068,15 @@ async fn handle_call<G: Fn(Vec<Effect>) + Send + Sync>(
20672068
)
20682069
.await?;
20692070
let pat = js_value_to_pattern(&linked_func_call);
2071+
use turbo_tasks::debug::ValueDebug;
2072+
println!(
2073+
"pathjoin {:?} {:?} {} {:?} {:?}",
2074+
get_traced_project_dir().await?,
2075+
*compile_time_info.environment().rendering().await?,
2076+
compile_time_info.environment().dbg().await?,
2077+
linked_func_call,
2078+
pat.describe_as_string(),
2079+
);
20702080
if !pat.has_constant_parts() {
20712081
let (args, hints) = explain_args(&args);
20722082
handler.span_warn_with_code(
@@ -2080,13 +2090,15 @@ async fn handle_call<G: Fn(Vec<Effect>) + Send + Sync>(
20802090
return Ok(());
20812091
}
20822092
}
2083-
if let Some(context_dir) = get_traced_project_dir().await? {
2084-
analysis.add_reference(
2085-
DirAssetReference::new(context_dir, Pattern::new(pat), make_issue_source())
2086-
.to_resolved()
2087-
.await?,
2088-
);
2089-
}
2093+
analysis.add_reference(
2094+
DirAssetReference::new(
2095+
get_traced_project_dir().await?,
2096+
Pattern::new(pat),
2097+
make_issue_source(),
2098+
)
2099+
.to_resolved()
2100+
.await?,
2101+
);
20902102
return Ok(());
20912103
}
20922104
JsValue::WellKnownFunction(WellKnownFunctionKind::ChildProcessSpawnMethod(name))
@@ -2130,12 +2142,10 @@ async fn handle_call<G: Fn(Vec<Effect>) + Send + Sync>(
21302142
if dynamic {
21312143
show_dynamic_warning = true;
21322144
}
2133-
if (!dynamic || !ignore_dynamic_requests)
2134-
&& let Some(context_dir) = get_traced_project_dir().await?
2135-
{
2145+
if !dynamic || !ignore_dynamic_requests {
21362146
analysis.add_reference(
21372147
FileSourceReference::new(
2138-
context_dir,
2148+
get_traced_project_dir().await?,
21392149
Pattern::new(pat),
21402150
collect_affecting_sources,
21412151
IssueSource::from_swc_offsets(
@@ -2371,17 +2381,15 @@ async fn handle_call<G: Fn(Vec<Effect>) + Send + Sync>(
23712381
.await?;
23722382
js_value_to_pattern(&linked_func_call)
23732383
};
2374-
if let Some(context_dir) = get_traced_project_dir().await? {
2375-
analysis.add_reference(
2376-
DirAssetReference::new(
2377-
context_dir,
2378-
Pattern::new(abs_pattern),
2379-
make_issue_source(),
2380-
)
2381-
.to_resolved()
2382-
.await?,
2383-
);
2384-
}
2384+
analysis.add_reference(
2385+
DirAssetReference::new(
2386+
get_traced_project_dir().await?,
2387+
Pattern::new(abs_pattern),
2388+
make_issue_source(),
2389+
)
2390+
.to_resolved()
2391+
.await?,
2392+
);
23852393
return Ok(());
23862394
}
23872395
}
@@ -2440,17 +2448,15 @@ async fn handle_call<G: Fn(Vec<Effect>) + Send + Sync>(
24402448
.await?;
24412449
js_value_to_pattern(&linked_func_call)
24422450
};
2443-
if let Some(context_dir) = get_traced_project_dir().await? {
2444-
analysis.add_reference(
2445-
DirAssetReference::new(
2446-
context_dir,
2447-
Pattern::new(abs_pattern),
2448-
make_issue_source(),
2449-
)
2450-
.to_resolved()
2451-
.await?,
2452-
);
2453-
}
2451+
analysis.add_reference(
2452+
DirAssetReference::new(
2453+
get_traced_project_dir().await?,
2454+
Pattern::new(abs_pattern),
2455+
make_issue_source(),
2456+
)
2457+
.to_resolved()
2458+
.await?,
2459+
);
24542460
return Ok(());
24552461
}
24562462
let (args, hints) = explain_args(&args);
@@ -2498,33 +2504,32 @@ async fn handle_call<G: Fn(Vec<Effect>) + Send + Sync>(
24982504
if args.len() == 2
24992505
&& let Some(JsValue::Object { parts, .. }) = args.get(1)
25002506
{
2501-
if let Some(context_dir) = get_traced_project_dir().await? {
2502-
let resolved_dirs = parts
2503-
.iter()
2504-
.filter_map(|object_part| match object_part {
2505-
ObjectPart::KeyValue(
2506-
JsValue::Constant(key),
2507-
JsValue::Array { items: dirs, .. },
2508-
) if key.as_str() == Some("includeDirs") => {
2509-
Some(dirs.iter().filter_map(|dir| dir.as_str()))
2510-
}
2511-
_ => None,
2512-
})
2513-
.flatten()
2514-
.map(|dir| {
2515-
DirAssetReference::new(
2516-
context_dir.clone(),
2517-
Pattern::new(Pattern::Constant(dir.into())),
2518-
make_issue_source(),
2519-
)
2520-
.to_resolved()
2521-
})
2522-
.try_join()
2523-
.await?;
2507+
let context_dir = get_traced_project_dir().await?;
2508+
let resolved_dirs = parts
2509+
.iter()
2510+
.filter_map(|object_part| match object_part {
2511+
ObjectPart::KeyValue(
2512+
JsValue::Constant(key),
2513+
JsValue::Array { items: dirs, .. },
2514+
) if key.as_str() == Some("includeDirs") => {
2515+
Some(dirs.iter().filter_map(|dir| dir.as_str()))
2516+
}
2517+
_ => None,
2518+
})
2519+
.flatten()
2520+
.map(|dir| {
2521+
DirAssetReference::new(
2522+
context_dir.clone(),
2523+
Pattern::new(Pattern::Constant(dir.into())),
2524+
make_issue_source(),
2525+
)
2526+
.to_resolved()
2527+
})
2528+
.try_join()
2529+
.await?;
25242530

2525-
for resolved_dir_ref in resolved_dirs {
2526-
analysis.add_reference(resolved_dir_ref);
2527-
}
2531+
for resolved_dir_ref in resolved_dirs {
2532+
analysis.add_reference(resolved_dir_ref);
25282533
}
25292534

25302535
return Ok(());

0 commit comments

Comments
 (0)