Repository: vercel/next.js
Pull request: Turbopack: hash next/font filename to shorten it

## What?

Hashes the filename of google fonts and truncates to 16 characters. This results in much shorter names as currently it was using some heuristic of 200 characters then add hashes but that didn't account for Turbopack itself adding content hashing to static files as well which could go over the limit.

This ensures the filenames stay much shorter.

- [x] Implement fix
- [x] Add test

Fixes #74432
Fixes PACK-3998

<!-- Thanks for opening a PR! Your contribution is much appreciated.
To make sure your PR is handled as smoothly as possible we request that you follow the checklist sections below.
Choose the right checklist for the change(s) that you're making:

## For Contributors

### Improving Documentation

- Run `pnpm prettier-fix` to fix formatting issues before opening the PR.
- Read the Docs Contribution Guide to ensure your contribution follows the docs guidelines: https://nextjs.org/docs/community/contribution-guide

### Adding or Updating Examples

- The "examples guidelines" are followed from our contributing doc https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md
- Make sure the linting passes by running `pnpm build && pnpm lint`. See https://github.com/vercel/next.js/blob/canary/contributing/repository/linting.md

### Fixing a bug

- Related issues linked using `fixes #number`
- Tests added. See: https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs
- Errors have a helpful link attached, see https://github.com/vercel/next.js/blob/canary/contributing.md

### Adding a feature

- Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR. (A discussion must be opened, see https://github.com/vercel/next.js/discussions/new?category=ideas)
- Related issues/discussions are linked using `fixes #number`
- e2e tests added (https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs)
- Documentation added
- Telemetry added. In case of a feature if it's used or not.
- Errors have a helpful link attached, see https://github.com/vercel/next.js/blob/canary/contributing.md


## For Maintainers

- Minimal description (aim for explaining to someone not on the team to understand the PR)
- When linking to a Slack thread, you might want to share details of the conclusion
- Link both the Linear (Fixes NEXT-xxx) and the GitHub issues
- Add review comments if necessary to explain to the reviewer the logic behind a change

### What?

### Why?

### How?

Closes NEXT-
Fixes #

-->


Changed files:
filename: crates/next-core/src/next_font/google/mod.rs
patch: @@ -14,6 +14,7 @@ use turbo_tasks_fs::{
     json::parse_json_with_source_context, DiskFileSystem, File, FileContent, FileSystem,
     FileSystemPath,
 };
+use turbo_tasks_hash::hash_xxh3_hash64;
 use turbopack::evaluate_context::node_evaluate_asset_context;
 use turbopack_core::{
     asset::AssetContent,
@@ -358,7 +359,7 @@ impl ImportMappingReplacement for NextFontGoogleFontFileReplacer {
         let ext = ext.with_context(|| format!("font url {} is missing an extension", &url))?;
 
         // remove dashes and dots as they might be used for the markers below.
-        let mut name = filename.replace(['-', '.'], "_");
+        let mut name = format!("{:016x}", hash_xxh3_hash64(filename.as_bytes()));
         if size_adjust {
             name.push_str("-s")
         }
@@ -367,8 +368,7 @@ impl ImportMappingReplacement for NextFontGoogleFontFileReplacer {
         }
 
         let font_virtual_path = next_js_file_path("internal/font/google".into())
-            .join(format!("/{}.{}", name, ext).into())
-            .truncate_file_name_with_hash_vc();
+            .join(format!("/{}.{}", name, ext).into());
 
         // doesn't seem ideal to download the font into a string, but probably doesn't
         // really matter either.

filename: test/e2e/next-font/index.test.ts
patch: @@ -410,6 +410,10 @@ describe('next/font', () => {
         .sort()
 
       for (const href of hrefs) {
+        // Check that font file path is not too long for Windows systems.
+        // Windows allows up to 256 characters but we check for 100 here
+        // because if it's over 100 it's already way too much.
+        expect(href.length).toBeLessThan(100)
         hrefMatchesFontWithSizeAdjust(href)
       }
 

filename: turbopack/crates/turbo-tasks-fs/src/lib.rs
patch: @@ -26,7 +26,7 @@ mod watcher;
 use std::{
     borrow::Cow,
     cmp::{min, Ordering},
-    fmt::{self, Debug, Display, Formatter, Write as _},
+    fmt::{self, Debug, Display, Formatter},
     fs::FileType,
     future::Future,
     io::{self, BufRead, ErrorKind},
@@ -63,9 +63,7 @@ use turbo_tasks::{
     ApplyEffectsContext, Completion, InvalidationReason, Invalidator, NonLocalValue, ReadRef,
     ResolvedVc, ValueToString, Vc,
 };
-use turbo_tasks_hash::{
-    hash_xxh3_hash128, hash_xxh3_hash64, DeterministicHash, DeterministicHasher,
-};
+use turbo_tasks_hash::{hash_xxh3_hash64, DeterministicHash, DeterministicHasher};
 use util::{extract_disk_access, join_path, normalize_path, sys_to_unix, unix_to_sys};
 pub use virtual_fs::VirtualFileSystem;
 use watcher::DiskWatcher;
@@ -1138,65 +1136,6 @@ impl FileSystemPath {
             (None, path_before_extension, extension)
         }
     }
-
-    /// Ensure the given filename (just the name, not the full path) is less than
-    /// [`MAX_SAFE_FILE_NAME_LENGTH`], truncating and suffixing with a hash as needed.
-    ///
-    /// If given a file extension, we will attempt to preserve that extension, failing with an error
-    /// if the extension is too long.
-    ///
-    /// A high-quality non-cryptographic 128-bit hash is used, which guarantees that there are no
-    /// collisions in the absence of malicious input. Given two unique input file paths, we
-    /// guarantee two unique output file paths.
-    ///
-    /// Note: We are hashing the _file name stem_, not the _contents_.
-    ///
-    /// If you have a [`Vc<FileSystemPath>`], call
-    /// [`FileSystemPath::truncate_file_name_with_hash_vc`] instead.
-    pub fn truncate_file_name_with_hash(&self) -> Result<Cow<'_, FileSystemPath>> {
-        let (parent, stem, extension) = self.split_file_stem_extension();
-        // length of the extension plus the dot ('.')
-        let extension_with_dot_len = extension.map(|ext| ext.len() + 1).unwrap_or(0);
-
-        // common case: file name is a safe length
-        if (stem.len() + extension_with_dot_len) <= MAX_SAFE_FILE_NAME_LENGTH {
-            return Ok(Cow::Borrowed(self));
-        }
-
-        // generate a 32-character hex-encoded hash
-        let hash = hash_xxh3_hash128(stem);
-        let hash_str = format!("{:01$x}", hash, std::mem::size_of::<u128>() * 2);
-
-        let remaining_len = MAX_SAFE_FILE_NAME_LENGTH
-            .checked_sub(
-                // +1 byte here for the underscore ('_') separator
-                hash_str.len() + extension_with_dot_len + 1,
-            )
-            .with_context(|| {
-                format!(
-                    "Unable to truncate {} because the file extension exceeds {} bytes",
-                    self.path,
-                    MAX_SAFE_FILE_NAME_LENGTH - 2
-                )
-            })?;
-
-        let mut path_str = String::with_capacity(
-            parent.map(|p| p.len() + 1).unwrap_or(0) + MAX_SAFE_FILE_NAME_LENGTH,
-        );
-        if let Some(parent) = parent {
-            // unwrap: write!() on a string should never fail
-            write!(path_str, "{parent}/").unwrap();
-        }
-        write!(path_str, "{}_{}", &stem[..remaining_len], hash_str).unwrap();
-        if let Some(extension) = extension {
-            write!(path_str, ".{extension}").unwrap();
-        }
-
-        Ok(Cow::Owned(FileSystemPath {
-            fs: self.fs,
-            path: RcStr::from(path_str),
-        }))
-    }
 }
 
 #[turbo_tasks::value(transparent)]
@@ -1399,16 +1338,6 @@ impl FileSystemPath {
         }
         Vc::cell(Some(file_stem.into()))
     }
-
-    /// See [`truncate_file_name_with_hash`]. Preserves the input [`Vc`] if no truncation was
-    /// performed.
-    #[turbo_tasks::function]
-    pub async fn truncate_file_name_with_hash_vc(self: Vc<Self>) -> Result<Vc<FileSystemPath>> {
-        Ok(match self.await?.truncate_file_name_with_hash()? {
-            Cow::Borrowed(_) => self,
-            Cow::Owned(path) => path.cell(),
-        })
-    }
 }
 
 impl Display for FileSystemPath {
@@ -2592,46 +2521,4 @@ mod tests {
         .await
         .unwrap()
     }
-
-    #[tokio::test]
-    async fn test_truncate_file_name_with_hash() {
-        crate::register();
-
-        turbo_tasks_testing::VcStorage::with(async {
-            let fs = Vc::upcast::<Box<dyn FileSystem>>(VirtualFileSystem::new());
-
-            let mut long_str = String::new();
-            for _i in 0..1000 {
-                long_str.push_str("long")
-            }
-
-            // does not change the path (returns exact same Vc) if the file name is short
-            let path = FileSystemPath::new_normalized(fs, format!("{long_str}/short.ext").into())
-                .resolve()
-                .await?;
-            assert_eq!(
-                path.truncate_file_name_with_hash_vc().resolve().await?,
-                path,
-            );
-
-            // truncates and adds hash so that the file name length equals MAX_SAFE_FILE_NAME_LENGTH
-            let path = FileSystemPath::new_normalized(fs, format!("path/{long_str}.ext").into());
-            let truncated_path = path.truncate_file_name_with_hash_vc().await?;
-            assert_eq!(
-                truncated_path.path,
-                "path/longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong\
-                longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong\
-                longlon_235b5eceeef9efad5b37fd5057ab8317.ext",
-            );
-            assert_eq!(truncated_path.file_name().len(), MAX_SAFE_FILE_NAME_LENGTH);
-
-            // an extension that's too long should fail
-            let path = FileSystemPath::new_normalized(fs, format!("path/foo.{long_str}").into());
-            assert!(path.truncate_file_name_with_hash_vc().await.is_err());
-
-            anyhow::Ok(())
-        })
-        .await
-        .unwrap()
-    }
 }