refactor(turbo-tasks-fs): Use ResolvedVc instead of Vc in structs (#7…

…0576)

This is a handmade refactor of `turbo-tasks-fs` to switch every struct's
field from `Vc<T>` to `ResolvedVc<T>`.

I chose `turbo-tasks-fs` because it's reasonably small, but very widely
used, and it has a representative mix of code patterns.

In the process of creating this PR, I wrote up a guide of the steps
required to refactor this, in the hope that we can automate most of
these steps for most code:
https://vercel.notion.site/Codemod-Notes-for-Vc-T-ResolvedVc-T-10ee06b059c480688ebffe5eb983db25

crates/next-api/src/pages.rs

@@ -1180,8 +1180,14 @@ impl PageEndpoint {
     }
 
     #[turbo_tasks::function]
-    fn client_relative_path(&self) -> Vc<FileSystemPathOption> {
-        Vc::cell(Some(self.pages_project.project().client_relative_path()))
+    async fn client_relative_path(&self) -> Result<Vc<FileSystemPathOption>> {
+        Ok(Vc::cell(Some(
+            self.pages_project
+                .project()
+                .client_relative_path()
+                .to_resolved()
+                .await?,
+        )))
     }
 }
 

crates/next-core/src/app_structure.rs

@@ -1330,9 +1330,9 @@ pub async fn get_global_metadata(
         };
 
         if dynamic {
-            *entry = Some(MetadataItem::Dynamic { path: file });
+            *entry = Some(MetadataItem::Dynamic { path: *file });
         } else {
-            *entry = Some(MetadataItem::Static { path: file });
+            *entry = Some(MetadataItem::Static { path: *file });
         }
         // TODO(WEB-952) handle symlinks in app dir
     }

crates/next-core/src/page_loader.rs

@@ -111,7 +111,7 @@ impl PageLoaderAsset {
                 .map(|chunk| {
                     Vc::upcast(ProxiedAsset::new(
                         *chunk,
-                        FileSystemPath::rebase(chunk.ident().path(), *rebase_path, root_path),
+                        FileSystemPath::rebase(chunk.ident().path(), **rebase_path, root_path),
                     ))
                 })
                 .collect();
@@ -131,7 +131,10 @@ fn page_loader_chunk_reference_description() -> Vc<RcStr> {
 impl OutputAsset for PageLoaderAsset {
     #[turbo_tasks::function]
     async fn ident(&self) -> Result<Vc<AssetIdent>> {
-        let root = self.rebase_prefix_path.await?.unwrap_or(self.server_root);
+        let root = self
+            .rebase_prefix_path
+            .await?
+            .map_or(self.server_root, |path| *path);
         Ok(AssetIdent::from_path(
             root.join(
                 format!(

crates/next-core/src/pages_structure.rs

@@ -117,33 +117,29 @@ pub async fn find_pages_structure(
     let pages_root = project_root
         .join("pages".into())
         .realpath()
-        .resolve()
+        .to_resolved()
         .await?;
-    let pages_root = Vc::<FileSystemPathOption>::cell(
-        if *pages_root.get_type().await? == FileSystemEntryType::Directory {
-            Some(pages_root)
+    let pages_root = if *pages_root.get_type().await? == FileSystemEntryType::Directory {
+        Some(pages_root)
+    } else {
+        let src_pages_root = project_root
+            .join("src/pages".into())
+            .realpath()
+            .to_resolved()
+            .await?;
+        if *src_pages_root.get_type().await? == FileSystemEntryType::Directory {
+            Some(src_pages_root)
         } else {
-            let src_pages_root = project_root
-                .join("src/pages".into())
-                .realpath()
-                .resolve()
-                .await?;
-            if *src_pages_root.get_type().await? == FileSystemEntryType::Directory {
-                Some(src_pages_root)
-            } else {
-                // If neither pages nor src/pages exists, we still want to generate
-                // the pages structure, but with no pages and default values for
-                // _app, _document and _error.
-                None
-            }
-        },
-    )
-    .resolve()
-    .await?;
+            // If neither pages nor src/pages exists, we still want to generate
+            // the pages structure, but with no pages and default values for
+            // _app, _document and _error.
+            None
+        }
+    };
 
     Ok(get_pages_structure_for_root_directory(
         project_root,
-        pages_root,
+        Vc::cell(pages_root),
         next_router_root,
         page_extensions,
     ))
@@ -202,7 +198,7 @@ async fn get_pages_structure_for_root_directory(
                     DirectoryEntry::Directory(dir_project_path) => match name.as_str() {
                         "api" => {
                             api_directory = Some(get_pages_structure_for_directory(
-                                dir_project_path,
+                                *dir_project_path,
                                 next_router_path.join(name.clone()),
                                 1,
                                 page_extensions,
@@ -212,7 +208,7 @@ async fn get_pages_structure_for_root_directory(
                             children.push((
                                 name,
                                 get_pages_structure_for_directory(
-                                    dir_project_path,
+                                    *dir_project_path,
                                     next_router_path.join(name.clone()),
                                     1,
                                     page_extensions,
@@ -231,7 +227,7 @@ async fn get_pages_structure_for_root_directory(
 
         Some(
             PagesDirectoryStructure {
-                project_path: *project_path,
+                project_path: **project_path,
                 next_router_path,
                 items: items.into_iter().map(|(_, v)| v).collect(),
                 children: children.into_iter().map(|(_, v)| v).collect(),
@@ -243,7 +239,7 @@ async fn get_pages_structure_for_root_directory(
     };
 
     let pages_path = if let Some(project_path) = *project_path {
-        project_path
+        *project_path
     } else {
         project_root.join("pages".into())
     };
@@ -338,7 +334,7 @@ async fn get_pages_structure_for_directory(
                         children.push((
                             name,
                             get_pages_structure_for_directory(
-                                *dir_project_path,
+                                **dir_project_path,
                                 next_router_path.join(name.clone()),
                                 position + 1,
                                 page_extensions,

turbopack/crates/node-file-trace/src/lib.rs

@@ -205,7 +205,7 @@ async fn add_glob_results(
     let result = result.await?;
     for entry in result.results.values() {
         if let DirectoryEntry::File(path) = entry {
-            let source = Vc::upcast(FileSource::new(*path));
+            let source = Vc::upcast(FileSource::new(**path));
             let module = asset_context
                 .process(
                     source,
@@ -224,7 +224,7 @@ async fn add_glob_results(
             Box::pin(add_glob_results(asset_context, result, list))
         }
         // Boxing for async recursion
-        recurse(asset_context, *result, list).await?;
+        recurse(asset_context, **result, list).await?;
     }
     Ok(())
 }

turbopack/crates/turbo-tasks-fs/examples/hash_directory.rs

@@ -76,12 +76,12 @@ async fn hash_directory(directory: Vc<FileSystemPath>) -> Result<Vc<RcStr>> {
             for entry in entries.values() {
                 match entry {
                     DirectoryEntry::File(path) => {
-                        let name = filename(*path).await?;
-                        hashes.insert(name, hash_file(*path).await?.clone_value());
+                        let name = filename(**path).await?;
+                        hashes.insert(name, hash_file(**path).await?.clone_value());
                     }
                     DirectoryEntry::Directory(path) => {
-                        let name = filename(*path).await?;
-                        hashes.insert(name, hash_directory(*path).await?.clone_value());
+                        let name = filename(**path).await?;
+                        hashes.insert(name, hash_directory(**path).await?.clone_value());
                     }
                     _ => {}
                 }

turbopack/crates/turbo-tasks-fs/examples/hash_glob.rs

@@ -72,11 +72,11 @@ async fn hash_glob_result(result: Vc<ReadGlobResult>) -> Result<Vc<RcStr>> {
     let mut hashes = BTreeMap::new();
     for (name, entry) in result.results.iter() {
         if let DirectoryEntry::File(path) = entry {
-            hashes.insert(name, hash_file(*path).await?.clone_value());
+            hashes.insert(name, hash_file(**path).await?.clone_value());
         }
     }
     for (name, result) in result.inner.iter() {
-        let hash = hash_glob_result(*result).await?;
+        let hash = hash_glob_result(**result).await?;
         if !hash.is_empty() {
             hashes.insert(name, hash.clone_value());
         }

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

@@ -1,6 +1,6 @@
 use anyhow::{bail, Result};
 use auto_hash_map::AutoMap;
-use turbo_tasks::{Completion, RcStr, ValueToString, Vc};
+use turbo_tasks::{Completion, RcStr, ResolvedVc, ValueToString, Vc};
 
 use crate::{
     DirectoryContent, DirectoryEntry, FileContent, FileMeta, FileSystem, FileSystemPath,
@@ -13,11 +13,11 @@ use crate::{
 /// Caveat: The `child_path` itself is not visible as a directory entry.
 #[turbo_tasks::value]
 pub struct AttachedFileSystem {
-    root_fs: Vc<Box<dyn FileSystem>>,
+    root_fs: ResolvedVc<Box<dyn FileSystem>>,
     // we turn this into a string because creating a FileSystemPath requires the filesystem which
     // we are creating (circular reference)
     child_path: RcStr,
-    child_fs: Vc<Box<dyn FileSystem>>,
+    child_fs: ResolvedVc<Box<dyn FileSystem>>,
 }
 
 #[turbo_tasks::value_impl]
@@ -27,7 +27,7 @@ impl AttachedFileSystem {
     #[turbo_tasks::function]
     pub async fn new(
         child_path: Vc<FileSystemPath>,
-        child_fs: Vc<Box<dyn FileSystem>>,
+        child_fs: ResolvedVc<Box<dyn FileSystem>>,
     ) -> Result<Vc<Self>> {
         let child_path = child_path.await?;
 
@@ -45,11 +45,11 @@ impl AttachedFileSystem {
     /// [FileSystem] or this [AttachedFileSystem].
     #[turbo_tasks::function]
     pub async fn convert_path(
-        self: Vc<Self>,
+        self: ResolvedVc<Self>,
         contained_path_vc: Vc<FileSystemPath>,
     ) -> Result<Vc<FileSystemPath>> {
         let contained_path = contained_path_vc.await?;
-        let self_fs: Vc<Box<dyn FileSystem>> = Vc::upcast(self);
+        let self_fs: ResolvedVc<Box<dyn FileSystem>> = ResolvedVc::upcast(self);
         let this = self.await?;
 
         match contained_path.fs {
@@ -89,12 +89,12 @@ impl AttachedFileSystem {
     /// on the [AttachedFileSystem]
     #[turbo_tasks::function]
     pub async fn get_inner_fs_path(
-        self: Vc<Self>,
+        self: ResolvedVc<Self>,
         path: Vc<FileSystemPath>,
     ) -> Result<Vc<FileSystemPath>> {
         let this = self.await?;
         let path = path.await?;
-        let self_fs: Vc<Box<dyn FileSystem>> = Vc::upcast(self);
+        let self_fs: ResolvedVc<Box<dyn FileSystem>> = ResolvedVc::upcast(self);
 
         if path.fs != self_fs {
             let self_fs_str = self_fs.to_string().await?;
@@ -144,10 +144,10 @@ impl FileSystem for AttachedFileSystem {
             use DirectoryEntry::*;
 
             let entry = match *entry {
-                File(path) => File(self.convert_path(path)),
-                Directory(path) => Directory(self.convert_path(path)),
-                Symlink(path) => Symlink(self.convert_path(path)),
-                Other(path) => Other(self.convert_path(path)),
+                File(path) => File(self.convert_path(*path).to_resolved().await?),
+                Directory(path) => Directory(self.convert_path(*path).to_resolved().await?),
+                Symlink(path) => Symlink(self.convert_path(*path).to_resolved().await?),
+                Other(path) => Other(self.convert_path(*path).to_resolved().await?),
                 Error => Error,
             };
 

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

@@ -1,4 +1,5 @@
 use anyhow::{bail, Result};
+use auto_hash_map::AutoMap;
 use include_dir::{Dir, DirEntry};
 use turbo_tasks::{Completion, RcStr, ValueToString, Vc};
 
@@ -46,29 +47,26 @@ impl FileSystem for EmbeddedFileSystem {
             (_, None) => return Ok(DirectoryContent::NotFound.cell()),
         };
 
-        let entries = dir
-            .entries()
-            .iter()
-            .map(|e| {
-                let entry_name: RcStr = e
-                    .path()
-                    .file_name()
-                    .unwrap_or_default()
-                    .to_string_lossy()
-                    .into();
-                let entry_path = path.join(entry_name.clone());
-
-                (
-                    entry_name,
-                    match e {
-                        DirEntry::Dir(_) => DirectoryEntry::Directory(entry_path),
-                        DirEntry::File(_) => DirectoryEntry::File(entry_path),
-                    },
-                )
-            })
-            .collect();
-
-        Ok(DirectoryContent::new(entries))
+        let mut converted_entries = AutoMap::new();
+        for e in dir.entries() {
+            let entry_name: RcStr = e
+                .path()
+                .file_name()
+                .unwrap_or_default()
+                .to_string_lossy()
+                .into();
+            let entry_path = path.join(entry_name.clone()).to_resolved().await?;
+
+            converted_entries.insert(
+                entry_name,
+                match e {
+                    DirEntry::Dir(_) => DirectoryEntry::Directory(entry_path),
+                    DirEntry::File(_) => DirectoryEntry::File(entry_path),
+                },
+            );
+        }
+
+        Ok(DirectoryContent::new(converted_entries))
     }
 
     #[turbo_tasks::function]