Skip to content

Commit ec62d6a

Browse files
sokravercel-fleet-prod[bot]mischnic
authored
fix(turbopack): add get_relative_request_to for paths used as module requests (#98497)
### What? Adds `get_relative_request_to`, next to `get_relative_path_to`, for callers that need a relative path they can hand to the resolver as a module request. It returns `./c`, `../c` or `.`, where `get_relative_path_to` returns `c`, `../c` or `.`. Fixes `import.meta.glob`'s `base`-relative keys, which currently come back without the leading `./`: `{ base: '../outside' }` produces `one.js` where Vite (and our own docs, and the existing execution fixture) expect `./one.js`. That fixture is failing on `canary` today; it passes here unchanged. ### Why? The `./` prefix is not decoration — it is what makes a string a *relative* request. Without it, `c` resolves as the package `c` rather than the file next to `from`. `get_relative_path_to` stopped emitting the prefix for a path below `from`, and every caller that builds a request had to add it back by hand. Eight of them do, each with the same three lines. That works, but it puts the requirement in the call sites instead of the API, and nothing tells a new caller that it exists — so the ninth request-building call site, `import.meta.glob`'s `base`-relative key, was written without it and silently produced the wrong keys. The two changes were authored independently and each was green on its own; only together do they produce the bug, which is why CI caught it on `canary` rather than on either pull request. Rather than add a tenth hand-written prefix, this makes "give me a request" something you can ask for. The failure mode it removes is the silent one: a caller that forgets is no longer possible, because the choice is now the function you call. ### How? `get_relative_request_to` shares its traversal with `get_relative_path_to` through one private helper, so the two can't drift; the only difference is whether a result that would be bare gets the prefix. `get_relative_path_to` keeps its signature and behaviour exactly, including the fast path that returns `target` by reference when `from` is empty — the request form can't borrow there, and the unit tests assert that difference so the fast path isn't reused by mistake. `FileSystemPath` gains the matching method. The eight hand-written prefixes now call the new function and drop their snippets. Deciding which call sites to convert needed care, because the callers of `get_relative_path_to` genuinely want different things, and the ones that want a plain path deliberately keep it: - source-map paths reported back to the client, - the Next.js template expander, whose invariant check expects the bare form, - the paths written into NFT files, which are data rather than requests. Two locals that hold requests rather than paths are renamed accordingly, and one leftover `strip_prefix("./")` is removed — it had become a no-op undoing a prefix that is no longer produced. ### Testing The `import-meta-glob-relative` execution fixture already asserts the correct keys and currently fails, so the fix is verified by that fixture passing **unmodified** — its expectations were right all along. The unit test that covered these cases before the prefix went away is restored, now run against both functions and against `FileSystemPath`, which had been left with no coverage of either method. Its original assertions turn out to describe the request form exactly, so they pin that `get_relative_request_to` reproduces the earlier contract rather than merely resembling it. Beyond that: the full `turbopack-tests` suite, and the `next-server-nft` and `next-taskless` tests, which are what would break if a call site that wants a plain path had been converted by mistake. <!-- NEXT_JS_LLM --> <!-- fleet 3acc7e63-52a8-4472-89cd-b1bbb8ad59fa --> Co-authored-by: vercel-fleet-prod[bot] <318278635+vercel-fleet-prod[bot]@users.noreply.github.com> Co-authored-by: Tobias Koppers <1365881+sokra@users.noreply.github.com> Co-authored-by: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com>
1 parent 1329212 commit ec62d6a

9 files changed

Lines changed: 200 additions & 62 deletions

File tree

‎crates/next-core/src/next_shared/webpack_rules/babel.rs‎

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -353,15 +353,11 @@ pub async fn resolve_babel_plugin_react_compiler(
353353

354354
// The relative path should only ever fail to resolve when the `fs` is different, which should
355355
// only happen due to eventual consistency.
356-
let plugin_path = project_path
357-
.get_relative_path_to(&source.ident().await?.path.parent())
358-
.context("failed to resolve relative path for react compiler plugin")?;
359-
360-
Ok(Some(if plugin_path.starts_with("../") {
361-
plugin_path
362-
} else {
363-
RcStr::from(format!("./{plugin_path}"))
364-
}))
356+
Ok(Some(
357+
project_path
358+
.get_relative_request_to(&source.ident().await?.path.parent())
359+
.context("failed to resolve relative path for react compiler plugin")?,
360+
))
365361
}
366362

367363
#[turbo_tasks::value]

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

Lines changed: 125 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,9 @@ use turbo_tasks::{
1212
turbobail, turbofmt,
1313
};
1414
use turbo_tasks_hash::HashAlgorithm;
15-
use turbo_unix_path::{get_parent_path, get_relative_path_to, join_path, normalize_path};
15+
use turbo_unix_path::{
16+
get_parent_path, get_relative_path_to, get_relative_request_to, join_path, normalize_path,
17+
};
1618

1719
use crate::{
1820
DirectoryContent, DirectoryEntry, FileContent, FileJsonContent, FileMeta, FileSystem,
@@ -97,8 +99,13 @@ impl FileSystemPath {
9799
}
98100
}
99101

100-
/// Returns a unix-style path of `other` relative to `self`. Supports traversing upwards (`../`)
101-
/// within the filesystem.
102+
/// Returns a unix-style path of `other` relative to `self`, as a plain path: `dir/file.js`,
103+
/// `../file.js`, or `.`. Supports traversing upwards (`../`) within the filesystem.
104+
///
105+
/// Returns [`None`] when the two are on different filesystems.
106+
///
107+
/// The result is not prefixed with `./`, so it is a path and not a module request. Use
108+
/// [`FileSystemPath::get_relative_request_to`] to build an import specifier.
102109
pub fn get_relative_path_to(&self, other: &FileSystemPath) -> Option<RcStr> {
103110
if self.fs != other.fs {
104111
return None;
@@ -111,6 +118,25 @@ impl FileSystemPath {
111118
})
112119
}
113120

121+
/// Returns a unix-style path of `other` relative to `self`, as an explicitly relative module
122+
/// request: `./dir/file.js`, `../file.js`, or `.`. Supports traversing upwards (`../`) within
123+
/// the filesystem.
124+
///
125+
/// Returns [`None`] when the two are on different filesystems.
126+
///
127+
/// The `./` prefix is what makes the result a relative request rather than a reference to a
128+
/// package of that name. Use [`FileSystemPath::get_relative_path_to`] for a plain path.
129+
pub fn get_relative_request_to(&self, other: &FileSystemPath) -> Option<RcStr> {
130+
if self.fs != other.fs {
131+
return None;
132+
}
133+
134+
Some(match get_relative_request_to(&self.path, &other.path) {
135+
Cow::Borrowed(path) => path.into(),
136+
Cow::Owned(path) => path.into(),
137+
})
138+
}
139+
114140
/// Returns the final component of the FileSystemPath, or an empty string
115141
/// for the root path.
116142
pub fn file_name(&self) -> &str {
@@ -768,6 +794,102 @@ mod tests {
768794
use super::*;
769795
use crate::VirtualFileSystem;
770796

797+
/// Builds two paths on the same filesystem and returns them.
798+
fn paths_on_one_fs(
799+
fs: ResolvedVc<Box<dyn FileSystem>>,
800+
from: &str,
801+
target: &str,
802+
) -> (FileSystemPath, FileSystemPath) {
803+
(
804+
FileSystemPath::new_normalized_unchecked(fs, from.into()),
805+
FileSystemPath::new_normalized_unchecked(fs, target.into()),
806+
)
807+
}
808+
809+
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
810+
async fn test_get_relative_path_to() {
811+
let tt = turbo_tasks::TurboTasks::new(TurboTasksBackend::new(
812+
BackendOptions::default(),
813+
noop_backing_storage(),
814+
));
815+
tt.run_once(async move {
816+
let fs = Vc::upcast::<Box<dyn FileSystem>>(VirtualFileSystem::new())
817+
.to_resolved()
818+
.await?;
819+
820+
for (from, target, expected) in [
821+
("a/b/c", "a/b/c", "."),
822+
("a/c/d", "a/b/c", "../../b/c"),
823+
("", "a/b/c", "a/b/c"),
824+
("a/b", "a/b/c", "c"),
825+
("a/b/c", "", "../../.."),
826+
("a/b/c", "c/b/a", "../../../c/b/a"),
827+
] {
828+
let (from_path, target_path) = paths_on_one_fs(fs, from, target);
829+
assert_eq!(
830+
from_path.get_relative_path_to(&target_path).as_deref(),
831+
Some(expected),
832+
"{from:?} -> {target:?}"
833+
);
834+
}
835+
836+
// A path on another filesystem is not reachable relatively.
837+
let (from_path, _) = paths_on_one_fs(fs, "a/b", "a/b/c");
838+
let other_fs = Vc::upcast::<Box<dyn FileSystem>>(VirtualFileSystem::new())
839+
.to_resolved()
840+
.await?;
841+
let on_other_fs = FileSystemPath::new_normalized_unchecked(other_fs, rcstr!("a/b/c"));
842+
assert_eq!(from_path.get_relative_path_to(&on_other_fs), None);
843+
844+
anyhow::Ok(())
845+
})
846+
.await
847+
.unwrap();
848+
}
849+
850+
/// The cases this covers are the ones `get_relative_path_to` was asserted against before it
851+
/// stopped prefixing `./`, so they pin that the request form still produces them.
852+
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
853+
async fn test_get_relative_request_to() {
854+
let tt = turbo_tasks::TurboTasks::new(TurboTasksBackend::new(
855+
BackendOptions::default(),
856+
noop_backing_storage(),
857+
));
858+
tt.run_once(async move {
859+
let fs = Vc::upcast::<Box<dyn FileSystem>>(VirtualFileSystem::new())
860+
.to_resolved()
861+
.await?;
862+
863+
for (from, target, expected) in [
864+
("a/b/c", "a/b/c", "."),
865+
("a/c/d", "a/b/c", "../../b/c"),
866+
("", "a/b/c", "./a/b/c"),
867+
("a/b", "a/b/c", "./c"),
868+
("a/b/c", "", "../../.."),
869+
("a/b/c", "c/b/a", "../../../c/b/a"),
870+
] {
871+
let (from_path, target_path) = paths_on_one_fs(fs, from, target);
872+
assert_eq!(
873+
from_path.get_relative_request_to(&target_path).as_deref(),
874+
Some(expected),
875+
"{from:?} -> {target:?}"
876+
);
877+
}
878+
879+
// A path on another filesystem is not reachable relatively.
880+
let (from_path, _) = paths_on_one_fs(fs, "a/b", "a/b/c");
881+
let other_fs = Vc::upcast::<Box<dyn FileSystem>>(VirtualFileSystem::new())
882+
.to_resolved()
883+
.await?;
884+
let on_other_fs = FileSystemPath::new_normalized_unchecked(other_fs, rcstr!("a/b/c"));
885+
assert_eq!(from_path.get_relative_request_to(&on_other_fs), None);
886+
887+
anyhow::Ok(())
888+
})
889+
.await
890+
.unwrap();
891+
}
892+
771893
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
772894
async fn with_extension() {
773895
let tt = turbo_tasks::TurboTasks::new(TurboTasksBackend::new(

‎turbopack/crates/turbo-unix-path/src/lib.rs‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,13 +119,38 @@ pub fn normalize_request(str: &str) -> String {
119119
segments.join("/")
120120
}
121121

122+
/// Returns the path of `target` relative to `from`, as a plain path: `"c"`, `"../c"`, or `"."` when
123+
/// the two are equal.
124+
///
125+
/// The result is not prefixed with `./`, so it is a path and not a module request. Use
126+
/// [`get_relative_request_to`] to build an import specifier, where a bare `"c"` would be read as a
127+
/// package name rather than as a file next to `from`.
128+
///
122129
/// Returns `"."` by reference when the paths are identical, or `target` by reference when `from`
123130
/// is empty.
124131
pub fn get_relative_path_to<'a>(from: &str, target: &'a str) -> Cow<'a, str> {
125132
if from.is_empty() && !target.is_empty() {
126133
return Cow::Borrowed(target);
127134
}
128135

136+
relative_to(from, target, false)
137+
}
138+
139+
/// Returns the path of `target` relative to `from`, as an explicitly relative module request:
140+
/// `"./c"`, `"../c"`, or `"."` when the two are equal.
141+
///
142+
/// The `./` prefix is what makes the result a relative request: without it, `"c"` resolves as the
143+
/// package `c` instead of the file `c` next to `from`. Use [`get_relative_path_to`] when a plain
144+
/// path is wanted instead.
145+
///
146+
/// Returns `"."` by reference when the paths are identical.
147+
pub fn get_relative_request_to<'a>(from: &str, target: &'a str) -> Cow<'a, str> {
148+
relative_to(from, target, true)
149+
}
150+
151+
/// Shared by [`get_relative_path_to`] and [`get_relative_request_to`]; `explicitly_relative` adds
152+
/// the leading `./` that distinguishes a request from a path.
153+
fn relative_to<'a>(from: &str, target: &'a str, explicitly_relative: bool) -> Cow<'a, str> {
129154
fn split(s: &str) -> impl Iterator<Item = &str> {
130155
let mut iterator = s.split('/');
131156
if s.is_empty() {
@@ -147,6 +172,9 @@ pub fn get_relative_path_to<'a>(from: &str, target: &'a str) -> Cow<'a, str> {
147172
while from_segments.next().is_some() {
148173
result.push("..");
149174
}
175+
} else if explicitly_relative {
176+
// Nothing to walk up, so the path would be bare (`c`) without this.
177+
result.push(".");
150178
}
151179
for segment in target_segments {
152180
result.push(segment);
@@ -210,4 +238,28 @@ mod tests {
210238
assert_eq!(relative, expected);
211239
assert_eq!(matches!(relative, Cow::Borrowed(_)), borrowed);
212240
}
241+
242+
/// The same cases as [`test_get_relative_path_to`], so the two forms can be compared row by
243+
/// row. They differ only where the result would otherwise be a bare path, which is exactly
244+
/// where a request needs its `./`.
245+
#[rstest]
246+
#[case("a/b/c", "a/b/c", ".", true)]
247+
#[case("a/c/d", "a/b/c", "../../b/c", false)]
248+
#[case("", "a/b/c", "./a/b/c", false)]
249+
#[case("", "", ".", true)]
250+
#[case("a/b", "a/b/c", "./c", false)]
251+
#[case("a/b", "a/b/c/d", "./c/d", false)]
252+
#[case("a/b/c", "", "../../..", false)]
253+
#[case("a/b/c", "c/b/a", "../../../c/b/a", false)]
254+
#[case("file:///a/b/c", "file:///c/b/a", "../../../c/b/a", false)]
255+
fn test_get_relative_request_to(
256+
#[case] from: &str,
257+
#[case] target: &str,
258+
#[case] expected: &str,
259+
#[case] borrowed: bool,
260+
) {
261+
let relative = get_relative_request_to(from, target);
262+
assert_eq!(relative, expected);
263+
assert_eq!(matches!(relative, Cow::Borrowed(_)), borrowed);
264+
}
213265
}

‎turbopack/crates/turbopack-core/src/resolve/mod.rs‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3149,14 +3149,7 @@ async fn resolved(
31493149
path.parent(),
31503150
options,
31513151
options_value,
3152-
|package_path| {
3153-
let path = package_path.get_relative_path_to(&path_ref)?;
3154-
Some(if path.starts_with("../") {
3155-
path
3156-
} else {
3157-
RcStr::from(format!("./{path}"))
3158-
})
3159-
},
3152+
|package_path| package_path.get_relative_request_to(&path_ref),
31603153
query.clone(),
31613154
fragment.clone(),
31623155
)

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

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -544,24 +544,20 @@ impl ImportMetaGlobMap {
544544
// matching and user-visible specifiers have one explicit source of truth. The
545545
// module resolver resolves this logical request and tracks its symlink chain.
546546
let logical_path = scan_dir.join(scan_relative)?;
547-
let Some(origin_relative) = origin_path.get_relative_path_to(&logical_path)
547+
let Some(origin_relative) = origin_path.get_relative_request_to(&logical_path)
548548
else {
549549
bail!(
550550
"import.meta.glob: failed to compute relative path from origin to \
551551
matched file"
552552
);
553553
};
554-
let origin_relative = if origin_relative.starts_with("../") {
555-
origin_relative
556-
} else {
557-
RcStr::from(format!("./{origin_relative}"))
558-
};
559554

560555
// Compute the user-visible key of this entry.
561556
let key: RcStr = if let Some(key_base) = key_base {
562-
// Vite keys the result relative to `base` when it is provided.
557+
// Vite keys the result relative to `base` when it is provided, and its
558+
// keys are `./`-prefixed like the origin-relative ones below.
563559
// https://vite.dev/guide/features.html#base-path
564-
let Some(key) = key_base.get_relative_path_to(&logical_path) else {
560+
let Some(key) = key_base.get_relative_request_to(&logical_path) else {
565561
bail!(
566562
"import.meta.glob: failed to compute relative path from base to \
567563
matched file"
@@ -583,7 +579,7 @@ impl ImportMetaGlobMap {
583579
of the project to matched file"
584580
);
585581
};
586-
format!("/{}", relative.strip_prefix("./").unwrap_or(&relative)).into()
582+
format!("/{relative}").into()
587583
} else {
588584
origin_relative.clone()
589585
};

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

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -90,12 +90,11 @@ impl DirList {
9090
for (_, entry) in entries.iter().flat_map(|m| m.iter()) {
9191
match entry {
9292
DirectoryEntry::File(path) => {
93-
if let Some(relative_path) = root_val.get_relative_path_to(path) {
94-
// Webpack always chacks the RegExp against a path prefixed with `./`
95-
let relative_path = RcStr::from(format!("./{relative_path}"));
96-
if regex.is_match(&relative_path) {
97-
list.insert(relative_path, DirListEntry::File(path.clone()));
98-
}
93+
// Webpack always checks the RegExp against a path prefixed with `./`
94+
if let Some(relative_path) = root_val.get_relative_request_to(path)
95+
&& regex.is_match(&relative_path)
96+
{
97+
list.insert(relative_path, DirListEntry::File(path.clone()));
9998
}
10099
}
101100
DirectoryEntry::Directory(path) if recursive => {
@@ -195,14 +194,9 @@ impl RequireContextMap {
195194
let mut map = FxIndexMap::default();
196195

197196
for (context_relative, path) in list {
198-
let Some(origin_relative) = origin_path.get_relative_path_to(path) else {
197+
let Some(origin_relative) = origin_path.get_relative_request_to(path) else {
199198
bail!("invariant error: this was already checked in `list_dir`");
200199
};
201-
let origin_relative = if origin_relative.starts_with("../") {
202-
origin_relative
203-
} else {
204-
RcStr::from(format!("./{origin_relative}"))
205-
};
206200

207201
let request = Request::parse(origin_relative.clone().into())
208202
.to_resolved()

‎turbopack/crates/turbopack-nodejs/src/ecmascript/node/entry/chunk.rs‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ use std::io::Write;
22

33
use anyhow::Result;
44
use indoc::writedoc;
5-
use turbo_rcstr::RcStr;
65
use turbo_tasks::{ResolvedVc, ValueToString, Vc, turbobail};
76
use turbo_tasks_fs::{File, FileContent, FileSystemPath};
87
use turbopack_core::{
@@ -76,20 +75,15 @@ impl EcmascriptBuildNodeEntryChunk {
7675
let chunk_path = self.path().owned().await?;
7776
let chunk_directory = self.path().await?.parent();
7877
let runtime_path = self.runtime_chunk().path().owned().await?;
79-
let runtime_relative_path =
80-
if let Some(path) = chunk_directory.get_relative_path_to(&runtime_path) {
78+
let runtime_request =
79+
if let Some(path) = chunk_directory.get_relative_request_to(&runtime_path) {
8180
path
8281
} else {
8382
turbobail!(
8483
"cannot find a relative path from the chunk ({chunk_path}) to the runtime \
8584
chunk ({runtime_path})",
8685
);
8786
};
88-
let runtime_request = if runtime_relative_path.starts_with("../") {
89-
runtime_relative_path
90-
} else {
91-
RcStr::from(format!("./{runtime_relative_path}"))
92-
};
9387
let chunk_public_path = if let Some(path) = output_root.get_path_to(&chunk_path) {
9488
path
9589
} else {

‎turbopack/crates/turbopack/src/lib.rs‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -719,15 +719,10 @@ async fn process_default_internal(
719719
.to_resolved()
720720
.await?;
721721

722-
let loader_relative_path = execution_context_value
722+
let loader_request = execution_context_value
723723
.project_path
724-
.get_relative_path_to(&loader.loader)
724+
.get_relative_request_to(&loader.loader)
725725
.context("Loader path must be on project filesystem")?;
726-
let loader_request = if loader_relative_path.starts_with("../") {
727-
loader_relative_path
728-
} else {
729-
RcStr::from(format!("./{loader_relative_path}"))
730-
};
731726
let webpack_loader_item = WebpackLoaderItem {
732727
loader: loader_request,
733728
options: loader.options.clone(),

0 commit comments

Comments
 (0)