Skip to content

Commit bd802d1

Browse files
authored
[turbopack] Handle relative paths before matching in node file traces (#82539)
# Fix glob handling for NFT JSON asset includes and excludes The current webpack version uses the https://www.npmjs.com/package/glob package to match and notably only handles `../` when it appears in a prefix position ([ref](https://www.npmjs.com/package/glob#:~:text=..%20path%20portions%20are%20not%20handled%20unless%20they%20appear%20at%20the%20start%20of%20the%20pattern)), so we do the exact same thing here. This allows our read_glob traversal to only consider traversing 'down' from the root since we can preprocess the patterns to not include `../` or `./` Part-Of PACK-5219
1 parent 5bbd5dd commit bd802d1

File tree

4 files changed

+274
-61
lines changed

4 files changed

+274
-61
lines changed

crates/next-api/src/nft_json.rs

Lines changed: 256 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use anyhow::{Result, bail};
44
use serde_json::json;
55
use turbo_rcstr::RcStr;
66
use turbo_tasks::{
7-
ReadRef, ResolvedVc, TryFlatJoinIterExt, Vc,
7+
FxIndexMap, ReadRef, ResolvedVc, TryFlatJoinIterExt, TryJoinIterExt, Vc,
88
graph::{AdjacencyMap, GraphTraversal},
99
};
1010
use turbo_tasks_fs::{DirectoryEntry, File, FileSystem, FileSystemPath, glob::Glob};
@@ -102,6 +102,7 @@ async fn apply_includes(
102102
glob: Vc<Glob>,
103103
ident_folder: &FileSystemPath,
104104
) -> Result<BTreeSet<RcStr>> {
105+
debug_assert_eq!(project_root_path.fs, ident_folder.fs);
105106
// Read files matching the glob pattern from the project root
106107
let glob_result = project_root_path.read_glob(glob).await?;
107108

@@ -118,9 +119,10 @@ async fn apply_includes(
118119

119120
let file_path_ref = file_path;
120121
// Convert to relative path from ident_folder to the file
121-
if let Some(relative_path) = ident_folder.get_relative_path_to(file_path_ref) {
122-
result.insert(relative_path);
123-
}
122+
// unwrap is safe because project_root_path and ident_folder have the same filesystem
123+
// and paths produced by read_glob stay in the filesystem
124+
let relative_path = ident_folder.get_relative_path_to(file_path_ref).unwrap();
125+
result.insert(relative_path);
124126
}
125127

126128
for nested_result in glob_result.inner.values() {
@@ -163,9 +165,8 @@ impl Asset for NftJsonAsset {
163165
.chain(std::iter::once(chunk))
164166
.collect();
165167

168+
let project_path = this.project.project_path().owned().await?;
166169
let exclude_glob = if let Some(route) = &this.page_name {
167-
let project_path = this.project.project_path().await?;
168-
169170
if let Some(excludes_config) = output_file_tracing_excludes {
170171
let mut combined_excludes = BTreeSet::new();
171172

@@ -178,26 +179,37 @@ impl Asset for NftJsonAsset {
178179
{
179180
for pattern in patterns {
180181
if let Some(pattern_str) = pattern.as_str() {
181-
combined_excludes.insert(pattern_str);
182+
let (glob, root) =
183+
relativize_glob(pattern_str, project_path.clone())?;
184+
let glob = if root.path.is_empty() {
185+
glob.to_string()
186+
} else {
187+
format!("{root}/{glob}")
188+
};
189+
combined_excludes.insert(glob);
182190
}
183191
}
184192
}
185193
}
186194
}
187195

188-
let glob = Glob::new(
189-
format!(
190-
"{project_path}/{{{}}}",
191-
combined_excludes
192-
.iter()
193-
.copied()
194-
.collect::<Vec<_>>()
195-
.join(",")
196-
)
197-
.into(),
198-
);
199-
200-
Some(glob)
196+
if combined_excludes.is_empty() {
197+
None
198+
} else {
199+
let glob = Glob::new(
200+
format!(
201+
"{{{}}}",
202+
combined_excludes
203+
.iter()
204+
.map(|s| s.as_str())
205+
.collect::<Vec<_>>()
206+
.join(",")
207+
)
208+
.into(),
209+
);
210+
211+
Some(glob)
212+
}
201213
} else {
202214
None
203215
}
@@ -234,8 +246,8 @@ impl Asset for NftJsonAsset {
234246
// Apply outputFileTracingIncludes and outputFileTracingExcludes
235247
// Extract route from chunk path for pattern matching
236248
if let Some(route) = &this.page_name {
237-
let project_path = this.project.project_path().owned().await?;
238-
let mut combined_includes = BTreeSet::new();
249+
let mut combined_includes_by_root: FxIndexMap<FileSystemPath, Vec<&str>> =
250+
FxIndexMap::default();
239251

240252
// Process includes
241253
if let Some(includes_config) = output_file_tracing_includes
@@ -249,30 +261,29 @@ impl Asset for NftJsonAsset {
249261
{
250262
for pattern in patterns {
251263
if let Some(pattern_str) = pattern.as_str() {
252-
combined_includes.insert(pattern_str);
264+
let (glob, root) =
265+
relativize_glob(pattern_str, project_path.clone())?;
266+
combined_includes_by_root
267+
.entry(root)
268+
.or_default()
269+
.push(glob);
253270
}
254271
}
255272
}
256273
}
257274
}
258275

259276
// Apply includes - find additional files that match the include patterns
260-
if !combined_includes.is_empty() {
261-
let glob = Glob::new(
262-
format!(
263-
"{{{}}}",
264-
combined_includes
265-
.iter()
266-
.copied()
267-
.collect::<Vec<_>>()
268-
.join(",")
269-
)
270-
.into(),
271-
);
272-
let additional_files =
273-
apply_includes(project_path, glob, &ident_folder_in_project_fs).await?;
274-
result.extend(additional_files);
275-
}
277+
let includes = combined_includes_by_root
278+
.into_iter()
279+
.map(|(root, globs)| {
280+
let glob = Glob::new(format!("{{{}}}", globs.join(",")).into());
281+
apply_includes(root, glob, &ident_folder_in_project_fs)
282+
})
283+
.try_join()
284+
.await?;
285+
286+
result.extend(includes.into_iter().flatten());
276287
}
277288

278289
let json = json!({
@@ -284,6 +295,33 @@ impl Asset for NftJsonAsset {
284295
}
285296
}
286297

298+
/// The globs defined in the next.config.mjs are relative to the project root.
299+
/// The glob walker in turbopack is somewhat naive so we handle relative path directives first so
300+
/// traversal doesn't need to consider them and can just traverse 'down' the tree.
301+
/// The main alternative is to merge glob evaluation with directory traversal which is what the npm
302+
/// `glob` package does, but this would be a substantial rewrite.`
303+
fn relativize_glob(glob: &str, relative_to: FileSystemPath) -> Result<(&str, FileSystemPath)> {
304+
let mut relative_to = relative_to;
305+
let mut processed_glob = glob;
306+
loop {
307+
if let Some(stripped) = processed_glob.strip_prefix("../") {
308+
if relative_to.path.is_empty() {
309+
bail!(
310+
"glob '{glob}' is invalid, it has a prefix that navigates out of the project \
311+
root"
312+
);
313+
}
314+
relative_to = relative_to.parent();
315+
processed_glob = stripped;
316+
} else if let Some(stripped) = processed_glob.strip_prefix("./") {
317+
processed_glob = stripped;
318+
} else {
319+
break;
320+
}
321+
}
322+
Ok((processed_glob, relative_to))
323+
}
324+
287325
/// Walks the asset graph from multiple assets and collect all referenced
288326
/// assets, but filters out all client assets and glob matches.
289327
#[turbo_tasks::function]
@@ -341,3 +379,182 @@ async fn get_referenced_server_assets(
341379
.try_flat_join()
342380
.await
343381
}
382+
383+
#[cfg(test)]
384+
mod tests {
385+
use turbo_tasks::ResolvedVc;
386+
use turbo_tasks_backend::{BackendOptions, TurboTasksBackend, noop_backing_storage};
387+
use turbo_tasks_fs::{FileSystemPath, NullFileSystem};
388+
389+
use super::*;
390+
391+
fn create_test_fs_path(path: &str) -> FileSystemPath {
392+
crate::register();
393+
394+
FileSystemPath {
395+
fs: ResolvedVc::upcast(NullFileSystem {}.resolved_cell()),
396+
path: path.into(),
397+
}
398+
}
399+
400+
#[tokio::test]
401+
async fn test_relativize_glob_normal_patterns() {
402+
let tt = turbo_tasks::TurboTasks::new(TurboTasksBackend::new(
403+
BackendOptions::default(),
404+
noop_backing_storage(),
405+
));
406+
tt.run_once(async {
407+
// Test normal glob patterns without relative prefixes
408+
let base_path = create_test_fs_path("project/src");
409+
410+
let (glob, path) = relativize_glob("*.js", base_path.clone()).unwrap();
411+
assert_eq!(glob, "*.js");
412+
assert_eq!(path.path.as_str(), "project/src");
413+
414+
let (glob, path) = relativize_glob("components/**/*.tsx", base_path.clone()).unwrap();
415+
assert_eq!(glob, "components/**/*.tsx");
416+
assert_eq!(path.path.as_str(), "project/src");
417+
418+
let (glob, path) = relativize_glob("lib/utils.ts", base_path.clone()).unwrap();
419+
assert_eq!(glob, "lib/utils.ts");
420+
assert_eq!(path.path.as_str(), "project/src");
421+
Ok(())
422+
})
423+
.await
424+
.unwrap();
425+
}
426+
427+
#[tokio::test]
428+
async fn test_relativize_glob_current_directory_prefix() {
429+
let tt = turbo_tasks::TurboTasks::new(TurboTasksBackend::new(
430+
BackendOptions::default(),
431+
noop_backing_storage(),
432+
));
433+
tt.run_once(async {
434+
let base_path = create_test_fs_path("project/src");
435+
436+
// Single ./ prefix
437+
let (glob, path) = relativize_glob("./components/*.tsx", base_path.clone()).unwrap();
438+
assert_eq!(glob, "components/*.tsx");
439+
assert_eq!(path.path.as_str(), "project/src");
440+
441+
// Multiple ./ prefixes
442+
let (glob, path) = relativize_glob("././utils.js", base_path.clone()).unwrap();
443+
assert_eq!(glob, "utils.js");
444+
assert_eq!(path.path.as_str(), "project/src");
445+
446+
// ./ with complex glob
447+
let (glob, path) = relativize_glob("./lib/**/*.{js,ts}", base_path.clone()).unwrap();
448+
assert_eq!(glob, "lib/**/*.{js,ts}");
449+
assert_eq!(path.path.as_str(), "project/src");
450+
Ok(())
451+
})
452+
.await
453+
.unwrap();
454+
}
455+
456+
#[tokio::test]
457+
async fn test_relativize_glob_parent_directory_navigation() {
458+
let tt = turbo_tasks::TurboTasks::new(TurboTasksBackend::new(
459+
BackendOptions::default(),
460+
noop_backing_storage(),
461+
));
462+
tt.run_once(async {
463+
let base_path = create_test_fs_path("project/src/components");
464+
465+
// Single ../ prefix
466+
let (glob, path) = relativize_glob("../utils/*.js", base_path.clone()).unwrap();
467+
assert_eq!(glob, "utils/*.js");
468+
assert_eq!(path.path.as_str(), "project/src");
469+
470+
// Multiple ../ prefixes
471+
let (glob, path) = relativize_glob("../../lib/*.ts", base_path.clone()).unwrap();
472+
assert_eq!(glob, "lib/*.ts");
473+
assert_eq!(path.path.as_str(), "project");
474+
475+
// Complex navigation with glob
476+
let (glob, path) =
477+
relativize_glob("../../../external/**/*.json", base_path.clone()).unwrap();
478+
assert_eq!(glob, "external/**/*.json");
479+
assert_eq!(path.path.as_str(), "");
480+
Ok(())
481+
})
482+
.await
483+
.unwrap();
484+
}
485+
486+
#[tokio::test]
487+
async fn test_relativize_glob_mixed_prefixes() {
488+
let tt = turbo_tasks::TurboTasks::new(TurboTasksBackend::new(
489+
BackendOptions::default(),
490+
noop_backing_storage(),
491+
));
492+
tt.run_once(async {
493+
let base_path = create_test_fs_path("project/src/components");
494+
495+
// ../ followed by ./
496+
let (glob, path) = relativize_glob(".././utils/*.js", base_path.clone()).unwrap();
497+
assert_eq!(glob, "utils/*.js");
498+
assert_eq!(path.path.as_str(), "project/src");
499+
500+
// ./ followed by ../
501+
let (glob, path) = relativize_glob("./../lib/*.ts", base_path.clone()).unwrap();
502+
assert_eq!(glob, "lib/*.ts");
503+
assert_eq!(path.path.as_str(), "project/src");
504+
505+
// Multiple mixed prefixes
506+
let (glob, path) =
507+
relativize_glob("././../.././external/*.json", base_path.clone()).unwrap();
508+
assert_eq!(glob, "external/*.json");
509+
assert_eq!(path.path.as_str(), "project");
510+
Ok(())
511+
})
512+
.await
513+
.unwrap();
514+
}
515+
516+
#[tokio::test]
517+
async fn test_relativize_glob_error_navigation_out_of_root() {
518+
let tt = turbo_tasks::TurboTasks::new(TurboTasksBackend::new(
519+
BackendOptions::default(),
520+
noop_backing_storage(),
521+
));
522+
tt.run_once(async {
523+
// Test navigating out of project root with empty path
524+
let empty_path = create_test_fs_path("");
525+
let result = relativize_glob("../outside.js", empty_path);
526+
assert!(result.is_err());
527+
assert!(
528+
result
529+
.unwrap_err()
530+
.to_string()
531+
.contains("navigates out of the project root")
532+
);
533+
534+
// Test navigating too far up from a shallow path
535+
let shallow_path = create_test_fs_path("project");
536+
let result = relativize_glob("../../outside.js", shallow_path);
537+
assert!(result.is_err());
538+
assert!(
539+
result
540+
.unwrap_err()
541+
.to_string()
542+
.contains("navigates out of the project root")
543+
);
544+
545+
// Test multiple ../ that would go out of root
546+
let base_path = create_test_fs_path("a/b");
547+
let result = relativize_glob("../../../outside.js", base_path);
548+
assert!(result.is_err());
549+
assert!(
550+
result
551+
.unwrap_err()
552+
.to_string()
553+
.contains("navigates out of the project root")
554+
);
555+
Ok(())
556+
})
557+
.await
558+
.unwrap();
559+
}
560+
}

test/integration/build-trace-extra-entries/app/next.config.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,10 @@ module.exports = {
2121
},
2222
outputFileTracingIncludes: {
2323
'/index': ['include-me/**/*'],
24-
'/route1': ['include-me/**/*'],
24+
'/route1': ['./include-me/**/*'],
2525
},
2626
outputFileTracingExcludes: {
2727
'/index': ['public/exclude-me/**/*'],
28-
'/route1': ['public/exclude-me/**/*'],
28+
'/route1': ['./public/exclude-me/**/*'],
2929
},
3030
}

0 commit comments

Comments
 (0)