mirror of
https://github.com/hanzoai/next.js.git
synced 2026-09-24 17:00:51 +00:00
fix(turbopack): preserve resolveExtensions priority in read_matches fast path (#91856)
### What? Fix a bug where Turbopack ignored the priority order of `resolveExtensions` when both a platform-specific variant (e.g. `Component.web.tsx`) and a default variant (`Component.tsx`) exist for the same module path. The first-listed extension should always win, but it often didn't. ### Why? Turbopack's `read_matches` function iterates over constant alternatives extracted from a resolve pattern (one per extension in `resolveExtensions`). It uses `enumerate()` to assign a priority `index` to each alternative — lower index = higher priority — so the correct file is picked after sorting. The `until_end=true` branch (direct file existence check) correctly threaded `index` through. But the `until_end=false` branch (subdirectory descent, the path taken when the pattern contains a `/` before the dynamic suffix) had a **hardcoded `0`** instead of `index`: ```rust // before — all alternatives get the same priority nested.push((0, read_matches(fs_path.clone(), ...))); // after — priority matches the extension's position in resolveExtensions nested.push((index, read_matches(fs_path.clone(), ...))); ``` With all alternatives at priority 0, the tie-breaking sort was effectively arbitrary. In practice `Component.tsx` frequently beat `Component.web.tsx` even when `.web.tsx` was listed first, breaking platform-specific module overrides — a common React Native Web / Expo Web pattern. ### How? **One-line Rust fix** in `turbopack/crates/turbopack-core/src/resolve/pattern.rs`: replace the hardcoded `0` with the loop variable `index` in the `until_end=false` branch of the fast path. **Turbopack unit tests** (`turbopack-core`): - `test_custom_extensions_web_before_default` — `.web.tsx` wins when listed first in `resolveExtensions` - `test_custom_extensions_fallback_when_web_missing` — correctly falls back to `.tsx` when `.web.tsx` is absent - Extended `test_read_matches` with `extension_ordering` and `subpath_ordering` cases covering both the `until_end=true` and `until_end=false` branches **Next.js e2e test** (extended `test/e2e/app-dir/resolve-extensions/`): - Added `PlatformComponent.web.tsx` (renders `"hello web platform"`) and `PlatformComponent.tsx` (renders `"hello default platform"`) - `.web.tsx` is placed before `.tsx` in `resolveExtensions` in `next.config.js` (for both Turbopack and webpack) - New assertion: SSR'd HTML must contain `"hello web platform"` and must **not** contain `"hello default platform"` Fixes #91117 - [x] Tests added. See: https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs --------- Co-authored-by: Tobias Koppers <[email protected]> Co-authored-by: Claude <[email protected]>
This commit is contained in:
co-authored by
Tobias Koppers
Claude
parent
16935744fb
commit
73e654f678
@@ -0,0 +1,3 @@
|
||||
export default function PlatformComponent() {
|
||||
return <span>hello default platform</span>
|
||||
}
|
||||
@@ -0,0 +1,3 @@
|
||||
export default function PlatformComponent() {
|
||||
return <span>hello web platform</span>
|
||||
}
|
||||
@@ -1,12 +1,14 @@
|
||||
import image from './image'
|
||||
import Image from 'next/image'
|
||||
import Component from './component'
|
||||
import PlatformComponent from './PlatformComponent'
|
||||
|
||||
export default function Page() {
|
||||
return (
|
||||
<p>
|
||||
<Image src={image} alt="hello image 1" />
|
||||
<Component />
|
||||
<PlatformComponent />
|
||||
</p>
|
||||
)
|
||||
}
|
||||
|
||||
@@ -1,4 +1,14 @@
|
||||
const extensions = ['', '.png', '.tsx', '.ts', '.jsx', '.js', '.json']
|
||||
// .web.tsx intentionally placed before .tsx to verify resolveExtensions priority
|
||||
const extensions = [
|
||||
'',
|
||||
'.png',
|
||||
'.web.tsx',
|
||||
'.tsx',
|
||||
'.ts',
|
||||
'.jsx',
|
||||
'.js',
|
||||
'.json',
|
||||
]
|
||||
|
||||
/**
|
||||
* @type {import('next').NextConfig}
|
||||
|
||||
@@ -20,4 +20,16 @@ describe('turbo-resolve-extensions', () => {
|
||||
expect(text).toContain('hello world')
|
||||
expect(text).toContain('hello client')
|
||||
})
|
||||
|
||||
it('should respect resolveExtensions priority (.web.tsx before .tsx)', async () => {
|
||||
// When both PlatformComponent.web.tsx and PlatformComponent.tsx exist,
|
||||
// the .web.tsx variant must win because it appears first in resolveExtensions.
|
||||
// This guards against a Turbopack bug where the alternative index was
|
||||
// hardcoded to 0 in the fast path, collapsing all alternatives to the same
|
||||
// priority and making the winner non-deterministic (usually wrong).
|
||||
const res = await next.fetch('/')
|
||||
const html = await res.text()
|
||||
expect(html).toContain('hello web platform')
|
||||
expect(html).not.toContain('hello default platform')
|
||||
})
|
||||
})
|
||||
|
||||
@@ -3469,6 +3469,7 @@ mod tests {
|
||||
pattern: rcstr!("./foo.js").into(),
|
||||
enable_typescript_with_output_extension: true,
|
||||
fully_specified: false,
|
||||
custom_extensions: None,
|
||||
expected: vec![("./foo.js", "foo.ts")],
|
||||
})
|
||||
.await;
|
||||
@@ -3481,6 +3482,7 @@ mod tests {
|
||||
pattern: rcstr!("./foo").into(),
|
||||
enable_typescript_with_output_extension: true,
|
||||
fully_specified: false,
|
||||
custom_extensions: None,
|
||||
expected: vec![("./foo", "foo.ts")],
|
||||
})
|
||||
.await;
|
||||
@@ -3493,6 +3495,7 @@ mod tests {
|
||||
pattern: rcstr!("./posts").into(),
|
||||
enable_typescript_with_output_extension: true,
|
||||
fully_specified: false,
|
||||
custom_extensions: None,
|
||||
expected: vec![("./posts", "posts.ts")],
|
||||
})
|
||||
.await;
|
||||
@@ -3505,6 +3508,7 @@ mod tests {
|
||||
pattern: rcstr!("./bar.js").into(),
|
||||
enable_typescript_with_output_extension: true,
|
||||
fully_specified: false,
|
||||
custom_extensions: None,
|
||||
expected: vec![("./bar.js", "bar.js")],
|
||||
})
|
||||
.await;
|
||||
@@ -3517,6 +3521,7 @@ mod tests {
|
||||
pattern: rcstr!("./foo.ts").into(),
|
||||
enable_typescript_with_output_extension: true,
|
||||
fully_specified: false,
|
||||
custom_extensions: None,
|
||||
expected: vec![("./foo.ts", "foo.ts")],
|
||||
})
|
||||
.await;
|
||||
@@ -3530,6 +3535,7 @@ mod tests {
|
||||
pattern: rcstr!("./client#frag").into(),
|
||||
enable_typescript_with_output_extension: true,
|
||||
fully_specified: false,
|
||||
custom_extensions: None,
|
||||
expected: vec![("./client", "client.ts")],
|
||||
})
|
||||
.await;
|
||||
@@ -3543,6 +3549,7 @@ mod tests {
|
||||
pattern: rcstr!("./client#component.js").into(),
|
||||
enable_typescript_with_output_extension: true,
|
||||
fully_specified: false,
|
||||
custom_extensions: None,
|
||||
// Whether or not this request key is correct somewhat ambiguous. It depends on whether
|
||||
// or not we consider this fragment to be part of the request pattern
|
||||
expected: vec![("./client", "client#component.ts")],
|
||||
@@ -3558,6 +3565,7 @@ mod tests {
|
||||
pattern: rcstr!("./page#section").into(),
|
||||
enable_typescript_with_output_extension: true,
|
||||
fully_specified: false,
|
||||
custom_extensions: None,
|
||||
expected: vec![("./page", "page#section.ts")],
|
||||
})
|
||||
.await;
|
||||
@@ -3570,6 +3578,7 @@ mod tests {
|
||||
pattern: rcstr!("./client?q=s").into(),
|
||||
enable_typescript_with_output_extension: true,
|
||||
fully_specified: false,
|
||||
custom_extensions: None,
|
||||
expected: vec![("./client", "client.ts")],
|
||||
})
|
||||
.await;
|
||||
@@ -3590,6 +3599,7 @@ mod tests {
|
||||
]),
|
||||
enable_typescript_with_output_extension: true,
|
||||
fully_specified: false,
|
||||
custom_extensions: None,
|
||||
expected: vec![
|
||||
("./src/foo.js", "src/foo.ts"),
|
||||
("./src/bar.js", "src/bar.js"),
|
||||
@@ -3611,6 +3621,7 @@ mod tests {
|
||||
]),
|
||||
enable_typescript_with_output_extension: true,
|
||||
fully_specified: false,
|
||||
custom_extensions: None,
|
||||
expected: vec![
|
||||
("./src/bar.js", "src/bar.js"),
|
||||
("./src/bar", "src/bar.js"),
|
||||
@@ -3627,12 +3638,61 @@ mod tests {
|
||||
.await;
|
||||
}
|
||||
|
||||
/// Test that custom `resolveExtensions` ordering is respected:
|
||||
/// `.web.tsx` appears before `.tsx` in the list, so it must win when both exist.
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn test_custom_extensions_web_before_default() {
|
||||
resolve_relative_request_test(TestParams {
|
||||
files: vec!["Component.web.tsx", "Component.tsx"],
|
||||
pattern: rcstr!("./Component").into(),
|
||||
enable_typescript_with_output_extension: false,
|
||||
fully_specified: false,
|
||||
custom_extensions: Some(vec![
|
||||
rcstr!(".web.tsx"),
|
||||
rcstr!(".web.ts"),
|
||||
rcstr!(".web.jsx"),
|
||||
rcstr!(".web.js"),
|
||||
rcstr!(".tsx"),
|
||||
rcstr!(".ts"),
|
||||
rcstr!(".jsx"),
|
||||
rcstr!(".js"),
|
||||
]),
|
||||
expected: vec![("./Component", "Component.web.tsx")],
|
||||
})
|
||||
.await;
|
||||
}
|
||||
|
||||
/// Test that when `.web.tsx` doesn't exist, resolution falls back to `.tsx`.
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn test_custom_extensions_fallback_when_web_missing() {
|
||||
resolve_relative_request_test(TestParams {
|
||||
files: vec!["Component.tsx"],
|
||||
pattern: rcstr!("./Component").into(),
|
||||
enable_typescript_with_output_extension: false,
|
||||
fully_specified: false,
|
||||
custom_extensions: Some(vec![
|
||||
rcstr!(".web.tsx"),
|
||||
rcstr!(".web.ts"),
|
||||
rcstr!(".web.jsx"),
|
||||
rcstr!(".web.js"),
|
||||
rcstr!(".tsx"),
|
||||
rcstr!(".ts"),
|
||||
rcstr!(".jsx"),
|
||||
rcstr!(".js"),
|
||||
]),
|
||||
expected: vec![("./Component", "Component.tsx")],
|
||||
})
|
||||
.await;
|
||||
}
|
||||
|
||||
/// Parameters for resolve_relative_request_test
|
||||
struct TestParams<'a> {
|
||||
files: Vec<&'a str>,
|
||||
pattern: Pattern,
|
||||
enable_typescript_with_output_extension: bool,
|
||||
fully_specified: bool,
|
||||
/// Custom extensions list; when `None`, uses the default `[".ts", ".js", ".json"]`
|
||||
custom_extensions: Option<Vec<RcStr>>,
|
||||
expected: Vec<(&'a str, &'a str)>,
|
||||
}
|
||||
|
||||
@@ -3643,6 +3703,7 @@ mod tests {
|
||||
pattern,
|
||||
enable_typescript_with_output_extension,
|
||||
fully_specified,
|
||||
custom_extensions,
|
||||
expected,
|
||||
}: TestParams<'_>,
|
||||
) {
|
||||
@@ -3673,6 +3734,8 @@ mod tests {
|
||||
noop_backing_storage(),
|
||||
));
|
||||
|
||||
let custom_extensions_owned = custom_extensions;
|
||||
|
||||
tt.run_once(async move {
|
||||
#[turbo_tasks::value(transparent)]
|
||||
struct ResolveRelativeRequestOutput(Vec<(String, String)>);
|
||||
@@ -3683,6 +3746,7 @@ mod tests {
|
||||
pattern: Pattern,
|
||||
enable_typescript_with_output_extension: bool,
|
||||
fully_specified: bool,
|
||||
custom_extensions: Option<Vec<RcStr>>,
|
||||
) -> anyhow::Result<Vc<ResolveRelativeRequestOutput>> {
|
||||
let fs = DiskFileSystem::new(rcstr!("temp"), path);
|
||||
let lookup_path = fs.root().owned().await?;
|
||||
@@ -3692,6 +3756,7 @@ mod tests {
|
||||
pattern,
|
||||
enable_typescript_with_output_extension,
|
||||
fully_specified,
|
||||
custom_extensions,
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -3719,6 +3784,7 @@ mod tests {
|
||||
pattern,
|
||||
enable_typescript_with_output_extension,
|
||||
fully_specified,
|
||||
custom_extensions_owned,
|
||||
)
|
||||
.read_strongly_consistent()
|
||||
.await?;
|
||||
@@ -3737,12 +3803,15 @@ mod tests {
|
||||
pattern: Pattern,
|
||||
enable_typescript_with_output_extension: bool,
|
||||
fully_specified: bool,
|
||||
custom_extensions: Option<Vec<RcStr>>,
|
||||
) -> anyhow::Result<Vc<ResolveResult>> {
|
||||
let request = Request::parse(pattern.clone());
|
||||
|
||||
let extensions = custom_extensions
|
||||
.unwrap_or_else(|| vec![rcstr!(".ts"), rcstr!(".js"), rcstr!(".json")]);
|
||||
let mut options_value = node_esm_resolve_options(lookup_path.clone())
|
||||
.with_fully_specified(fully_specified)
|
||||
.with_extensions(vec![rcstr!(".ts"), rcstr!(".js"), rcstr!(".json")])
|
||||
.with_extensions(extensions)
|
||||
.owned()
|
||||
.await?;
|
||||
options_value.enable_typescript_with_output_extension =
|
||||
|
||||
@@ -1632,7 +1632,7 @@ pub async fn read_matches(
|
||||
continue;
|
||||
};
|
||||
nested.push((
|
||||
0,
|
||||
index,
|
||||
read_matches(
|
||||
fs_path.clone(),
|
||||
concat(&prefix, subpath).into(),
|
||||
@@ -2662,6 +2662,8 @@ mod tests {
|
||||
dynamic: Vec<String>,
|
||||
dynamic_file_suffix: Vec<String>,
|
||||
node_modules_dynamic: Vec<String>,
|
||||
extension_ordering: Vec<String>,
|
||||
subpath_ordering: Vec<String>,
|
||||
}
|
||||
|
||||
#[turbo_tasks::function(operation)]
|
||||
@@ -2704,7 +2706,7 @@ mod tests {
|
||||
.collect::<Vec<_>>();
|
||||
|
||||
let node_modules_dynamic = read_matches(
|
||||
root,
|
||||
root.clone(),
|
||||
rcstr!(""),
|
||||
false,
|
||||
Pattern::new(Pattern::Constant(rcstr!("node_modules")).or_any_nested_file()),
|
||||
@@ -2714,10 +2716,57 @@ mod tests {
|
||||
.map(|m| m.name().to_string())
|
||||
.collect::<Vec<_>>();
|
||||
|
||||
// Test: extension ordering is preserved (fast path, until_end=true)
|
||||
// When both Component.web.tsx and Component.tsx exist, the order of
|
||||
// alternatives determines which comes first in results.
|
||||
let extension_ordering = read_matches(
|
||||
root.clone(),
|
||||
rcstr!(""),
|
||||
false,
|
||||
Pattern::new(Pattern::Alternatives(vec![
|
||||
Pattern::Constant(rcstr!("extensions/Component")),
|
||||
Pattern::Constant(rcstr!("extensions/Component.web.tsx")),
|
||||
Pattern::Constant(rcstr!("extensions/Component.tsx")),
|
||||
])),
|
||||
)
|
||||
.await?
|
||||
.into_iter()
|
||||
.map(|m| m.name().to_string())
|
||||
.collect::<Vec<_>>();
|
||||
|
||||
// Test: subpath ordering is preserved (fast path, until_end=false)
|
||||
// When alternatives route to different subdirectories, the index ordering
|
||||
// must be respected. This exercises the fix for the hardcoded `0` bug.
|
||||
let subpath_ordering = read_matches(
|
||||
root.clone(),
|
||||
rcstr!(""),
|
||||
false,
|
||||
Pattern::new({
|
||||
let mut p = Pattern::Alternatives(vec![
|
||||
Pattern::Concatenation(vec![
|
||||
Pattern::Constant(rcstr!("prio/a/")),
|
||||
Pattern::Dynamic,
|
||||
]),
|
||||
Pattern::Concatenation(vec![
|
||||
Pattern::Constant(rcstr!("prio/b/")),
|
||||
Pattern::Dynamic,
|
||||
]),
|
||||
]);
|
||||
p.normalize();
|
||||
p
|
||||
}),
|
||||
)
|
||||
.await?
|
||||
.into_iter()
|
||||
.map(|m| m.name().to_string())
|
||||
.collect::<Vec<_>>();
|
||||
|
||||
Ok(ReadMatchesOutput {
|
||||
dynamic,
|
||||
dynamic_file_suffix,
|
||||
node_modules_dynamic,
|
||||
extension_ordering,
|
||||
subpath_ordering,
|
||||
}
|
||||
.cell())
|
||||
}
|
||||
@@ -2727,7 +2776,25 @@ mod tests {
|
||||
// node_modules shouldn't be matched by Dynamic here
|
||||
assert_eq!(
|
||||
matches.dynamic,
|
||||
&["index.js", "sub", "sub/", "sub/foo-a.js", "sub/foo-b.js"]
|
||||
&[
|
||||
"extensions",
|
||||
"extensions/",
|
||||
"extensions/Component.tsx",
|
||||
"extensions/Component.web.tsx",
|
||||
"index.js",
|
||||
"prio",
|
||||
"prio/",
|
||||
"prio/a",
|
||||
"prio/a/",
|
||||
"prio/a/Component.tsx",
|
||||
"prio/b",
|
||||
"prio/b/",
|
||||
"prio/b/Component.tsx",
|
||||
"sub",
|
||||
"sub/",
|
||||
"sub/foo-a.js",
|
||||
"sub/foo-b.js",
|
||||
]
|
||||
);
|
||||
|
||||
// basic dynamic file suffix
|
||||
@@ -2740,6 +2807,30 @@ mod tests {
|
||||
// want to enumerate the list of packages here.
|
||||
assert_eq!(matches.node_modules_dynamic, &["node_modules"]);
|
||||
|
||||
// extension ordering: .web.tsx (index 1) must come before .tsx (index 2)
|
||||
assert_eq!(
|
||||
matches.extension_ordering,
|
||||
&["extensions/Component.web.tsx", "extensions/Component.tsx",]
|
||||
);
|
||||
|
||||
// subpath ordering: prio/a/ alternatives (index 0) must come before prio/b/
|
||||
// alternatives (index 1). This verifies the fix for the hardcoded `0` bug in
|
||||
// the until_end=false branch of the fast path.
|
||||
assert!(
|
||||
matches
|
||||
.subpath_ordering
|
||||
.iter()
|
||||
.position(|s| s.starts_with("prio/a/"))
|
||||
.unwrap()
|
||||
< matches
|
||||
.subpath_ordering
|
||||
.iter()
|
||||
.position(|s| s.starts_with("prio/b/"))
|
||||
.unwrap(),
|
||||
"Expected prio/a/ results before prio/b/ results, got: {:?}",
|
||||
matches.subpath_ordering
|
||||
);
|
||||
|
||||
Ok(())
|
||||
})
|
||||
.await
|
||||
|
||||
Reference in New Issue
Block a user