Skip to content

Commit f6663b7

Browse files
bgwsokraclaude
authored
Turbopack: Dedupe our two filesystempath extension/extension_ref functions (#92048)
PR is AI-generated - `extension` and `extension_ref` do the same thing. - `extension_ref`'s API that returns an `Option` was better than `extension`'s API that returns a potentially-empty string. - In some places we were splitting into a stem and extension, and joining them back to a filename, but those places could just get the original path's filename. --------- Co-authored-by: Tobias Koppers <[email protected]> Co-authored-by: Claude <[email protected]> Co-authored-by: Benjamin Woodruff <[email protected]>
1 parent 2bf38b0 commit f6663b7

9 files changed

Lines changed: 26 additions & 34 deletions

File tree

crates/next-api/src/next_server_nft.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ impl Asset for ServerNftJsonAsset {
142142
let DirectoryEntry::File(file) = entry else {
143143
continue;
144144
};
145-
if file.extension() == "js" {
145+
if file.extension() == Some("js") {
146146
server_output_assets.push(
147147
base_dir
148148
.get_relative_path_to(file)

crates/next-core/src/next_app/metadata/image.rs

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ async fn dynamic_image_metadata_with_generator_source(
3232
) -> Result<Vc<Box<dyn Source>>> {
3333
let stem = path.file_stem();
3434
let stem = stem.unwrap_or_default();
35-
let ext = path.extension();
3635

3736
let hash = path
3837
.read()
@@ -44,7 +43,7 @@ async fn dynamic_image_metadata_with_generator_source(
4443
let sizes = if use_numeric_sizes {
4544
"data.width = size.width; data.height = size.height;".to_string()
4645
} else {
47-
let sizes = if ext == "svg" {
46+
let sizes = if path.has_extension(".svg") {
4847
"any"
4948
} else {
5049
"${size.width}x${size.height}"
@@ -86,7 +85,7 @@ async fn dynamic_image_metadata_with_generator_source(
8685
}}
8786
"#,
8887
exported_fields_excluding_default = exported_fields_excluding_default,
89-
resource_path = StringifyJs(&format!("./{stem}.{ext}")),
88+
resource_path = StringifyJs(&format!("./{}", path.file_name())),
9089
pathname_prefix = StringifyJs(&page.to_string()),
9190
page_segment = StringifyJs(stem),
9291
sizes = sizes,
@@ -110,7 +109,6 @@ async fn dynamic_image_metadata_without_generator_source(
110109
) -> Result<Vc<Box<dyn Source>>> {
111110
let stem = path.file_stem();
112111
let stem = stem.unwrap_or_default();
113-
let ext = path.extension();
114112

115113
let hash = path
116114
.read()
@@ -122,7 +120,7 @@ async fn dynamic_image_metadata_without_generator_source(
122120
let sizes = if use_numeric_sizes {
123121
"data.width = size.width; data.height = size.height;".to_string()
124122
} else {
125-
let sizes = if ext == "svg" {
123+
let sizes = if path.has_extension(".svg") {
126124
"any"
127125
} else {
128126
"${size.width}x${size.height}"
@@ -158,7 +156,7 @@ async fn dynamic_image_metadata_without_generator_source(
158156
}}
159157
"#,
160158
exported_fields_excluding_default = exported_fields_excluding_default,
161-
resource_path = StringifyJs(&format!("./{stem}.{ext}")),
159+
resource_path = StringifyJs(&format!("./{}", path.file_name())),
162160
pathname_prefix = StringifyJs(&page.to_string()),
163161
page_segment = StringifyJs(stem),
164162
sizes = sizes,

crates/next-core/src/next_app/metadata/mod.rs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -92,11 +92,11 @@ pub(crate) async fn get_content_type(path: FileSystemPath) -> Result<String> {
9292
let mut ext = path.extension();
9393

9494
let name = stem.unwrap_or_default();
95-
if ext == "jpg" {
96-
ext = "jpeg"
95+
if ext == Some("jpg") {
96+
ext = Some("jpeg");
9797
}
9898

99-
if name == "favicon" && ext == "ico" {
99+
if name == "favicon" && ext == Some("ico") {
100100
return Ok("image/x-icon".to_string());
101101
}
102102
if name == "sitemap" {
@@ -109,7 +109,9 @@ pub(crate) async fn get_content_type(path: FileSystemPath) -> Result<String> {
109109
return Ok("application/manifest+json".to_string());
110110
}
111111

112-
if ext == "png" || ext == "jpeg" || ext == "ico" || ext == "svg" {
112+
if let Some(ext) = ext
113+
&& matches!(ext, "png" | "jpeg" | "ico" | "svg")
114+
{
113115
return Ok(mime_guess::from_ext(ext)
114116
.first_or_octet_stream()
115117
.to_string());

crates/next-core/src/next_app/metadata/route.rs

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,6 @@ async fn static_route_source(mode: NextMode, path: FileSystemPath) -> Result<Vc<
215215
async fn dynamic_text_route_source(path: FileSystemPath) -> Result<Vc<Box<dyn Source>>> {
216216
let stem = path.file_stem();
217217
let stem = stem.unwrap_or_default();
218-
let ext = path.extension();
219218

220219
let content_type = get_content_type(path.clone()).await?;
221220

@@ -250,7 +249,7 @@ async fn dynamic_text_route_source(path: FileSystemPath) -> Result<Vc<Box<dyn So
250249
251250
export * from {resource_path}
252251
"#,
253-
resource_path = StringifyJs(&format!("./{stem}.{ext}")),
252+
resource_path = StringifyJs(&format!("./{}", path.file_name())),
254253
content_type = StringifyJs(&content_type),
255254
file_type = StringifyJs(&stem),
256255
cache_control = StringifyJs(CACHE_HEADER_REVALIDATE),
@@ -270,7 +269,6 @@ async fn dynamic_sitemap_route_with_generate_source(
270269
) -> Result<Vc<Box<dyn Source>>> {
271270
let stem = path.file_stem();
272271
let stem = stem.unwrap_or_default();
273-
let ext = path.extension();
274272
let content_type = get_content_type(path.clone()).await?;
275273

276274
let code = formatdoc! {
@@ -340,7 +338,7 @@ async fn dynamic_sitemap_route_with_generate_source(
340338
return params
341339
}}
342340
"#,
343-
resource_path = StringifyJs(&format!("./{stem}.{ext}")),
341+
resource_path = StringifyJs(&format!("./{}", path.file_name())),
344342
content_type = StringifyJs(&content_type),
345343
file_type = StringifyJs(&stem),
346344
cache_control = StringifyJs(CACHE_HEADER_REVALIDATE),
@@ -360,7 +358,6 @@ async fn dynamic_sitemap_route_without_generate_source(
360358
) -> Result<Vc<Box<dyn Source>>> {
361359
let stem = path.file_stem();
362360
let stem = stem.unwrap_or_default();
363-
let ext = path.extension();
364361
let content_type = get_content_type(path.clone()).await?;
365362

366363
let code = formatdoc! {
@@ -391,7 +388,7 @@ async fn dynamic_sitemap_route_without_generate_source(
391388
392389
export * from {resource_path}
393390
"#,
394-
resource_path = StringifyJs(&format!("./{stem}.{ext}")),
391+
resource_path = StringifyJs(&format!("./{}", path.file_name())),
395392
content_type = StringifyJs(&content_type),
396393
file_type = StringifyJs(&stem),
397394
cache_control = StringifyJs(CACHE_HEADER_REVALIDATE),
@@ -423,7 +420,6 @@ async fn dynamic_image_route_with_metadata_source(
423420
) -> Result<Vc<Box<dyn Source>>> {
424421
let stem = path.file_stem();
425422
let stem = stem.unwrap_or_default();
426-
let ext = path.extension();
427423

428424
let code = formatdoc! {
429425
r#"
@@ -478,7 +474,7 @@ async fn dynamic_image_route_with_metadata_source(
478474
return staticParams
479475
}}
480476
"#,
481-
resource_path = StringifyJs(&format!("./{stem}.{ext}")),
477+
resource_path = StringifyJs(&format!("./{}", path.file_name())),
482478
};
483479

484480
let file = File::from(code);
@@ -495,7 +491,6 @@ async fn dynamic_image_route_without_metadata_source(
495491
) -> Result<Vc<Box<dyn Source>>> {
496492
let stem = path.file_stem();
497493
let stem = stem.unwrap_or_default();
498-
let ext = path.extension();
499494

500495
let code = formatdoc! {
501496
r#"
@@ -512,7 +507,7 @@ async fn dynamic_image_route_without_metadata_source(
512507
513508
export * from {resource_path}
514509
"#,
515-
resource_path = StringifyJs(&format!("./{stem}.{ext}")),
510+
resource_path = StringifyJs(&format!("./{}", path.file_name())),
516511
};
517512

518513
let file = File::from(code);

crates/next-core/src/next_server/resolve.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ impl AfterResolvePlugin for ExternalCjsModulesResolvePlugin {
171171
) -> Result<FileType> {
172172
// node.js only supports these file extensions
173173
// mjs is an esm module and we can't bundle that yet
174-
Ok(match raw_fs_path.extension_ref() {
174+
Ok(match raw_fs_path.extension() {
175175
Some("cjs" | "node" | "json") => FileType::CommonJs,
176176
Some("mjs") => FileType::EcmaScriptModule,
177177
Some("js") => {

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

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1490,15 +1490,15 @@ impl FileSystemPath {
14901490

14911491
/// Returns true if this path has the given extension
14921492
///
1493-
/// slightly faster than `self.extension_ref() == Some(extension)` as we can simply match a
1493+
/// slightly faster than `self.extension() == Some(extension)` as we can simply match a
14941494
/// suffix
14951495
pub fn has_extension(&self, extension: &str) -> bool {
14961496
debug_assert!(!extension.contains('/') && extension.starts_with('.'));
14971497
self.path.ends_with(extension)
14981498
}
14991499

15001500
/// Returns the extension (without a leading `.`)
1501-
pub fn extension_ref(&self) -> Option<&str> {
1501+
pub fn extension(&self) -> Option<&str> {
15021502
let (_, extension) = self.split_extension();
15031503
extension
15041504
}
@@ -1676,10 +1676,6 @@ impl FileSystemPath {
16761676
*self.fs
16771677
}
16781678

1679-
pub fn extension(&self) -> &str {
1680-
self.extension_ref().unwrap_or_default()
1681-
}
1682-
16831679
pub fn is_inside(&self, other: &FileSystemPath) -> bool {
16841680
self.is_inside_ref(other)
16851681
}

turbopack/crates/turbopack-browser/src/chunking_context.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -653,7 +653,7 @@ impl ChunkingContext for BrowserChunkingContext {
653653
.as_ref()
654654
.context("Missing content when trying to generate the content hash for static asset")?;
655655
let short_hash = &hash[..length as usize];
656-
let asset_path = match source_path.extension_ref() {
656+
let asset_path = match source_path.extension() {
657657
Some(ext) => format!(
658658
"{basename}.{short_hash}.{ext}",
659659
basename = &basename[..basename.len() - ext.len() - 1],

turbopack/crates/turbopack-image/src/process/mod.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ fn result_to_issue<T>(source: ResolvedVc<Box<dyn Source>>, result: Result<T>) ->
125125
fn load_image(
126126
path: ResolvedVc<Box<dyn Source>>,
127127
bytes: &[u8],
128-
extension: &str,
128+
extension: Option<&str>,
129129
) -> Option<(ImageBuffer, Option<ImageFormat>)> {
130130
result_to_issue(path, load_image_internal(path, bytes, extension))
131131
}
@@ -140,15 +140,16 @@ enum ImageBuffer {
140140
fn load_image_internal(
141141
image: ResolvedVc<Box<dyn Source>>,
142142
bytes: &[u8],
143-
extension: &str,
143+
extension: Option<&str>,
144144
) -> Result<(ImageBuffer, Option<ImageFormat>)> {
145145
let reader = image::ImageReader::new(Cursor::new(&bytes));
146146
let mut reader = reader
147147
.with_guessed_format()
148148
.context("unable to determine image format from file content")?;
149149
let mut format = reader.format();
150150
if format.is_none()
151-
&& let Some(new_format) = extension_to_image_format(extension)
151+
&& let Some(ext) = extension
152+
&& let Some(new_format) = extension_to_image_format(ext)
152153
{
153154
format = Some(new_format);
154155
reader.set_format(new_format);
@@ -350,7 +351,7 @@ pub async fn get_meta_data(
350351
let path = image.ident().path().await?;
351352
let extension = path.extension();
352353

353-
if extension == "svg" {
354+
if extension == Some("svg") {
354355
let content = result_to_issue(
355356
image,
356357
std::str::from_utf8(&bytes).context("Input image is not valid utf-8"),

turbopack/crates/turbopack-nodejs/src/chunking_context.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -494,7 +494,7 @@ impl ChunkingContext for NodeJsChunkingContext {
494494
.as_ref()
495495
.context("Missing content when trying to generate the content hash for static asset")?;
496496
let short_hash = &hash[..length as usize];
497-
let asset_path = match source_path.extension_ref() {
497+
let asset_path = match source_path.extension() {
498498
Some(ext) => format!(
499499
"{basename}.{short_hash}.{ext}",
500500
basename = &basename[..basename.len() - ext.len() - 1],

0 commit comments

Comments
 (0)