Turbopack: NFT followups (#72517)

Closes PACK-3421

Address PR feedback from #72305

When working on this, I uncovered another bug which will be handled in a
different PR.

Changes:
- Don't rely on `DiskFileSystem` but only via file paths and
`is_inside()`
- Put the `RebasedAsset` for externals on the `[project]/` fs (this way,
they aren't emitted either), i.e. don't perform any rebasing at all and
just reference the files where they are

---------

Co-authored-by: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com>

crates/next-api/src/app.rs

@@ -1400,10 +1400,8 @@ impl AppEndpoint {
                 {
                     server_assets.insert(ResolvedVc::upcast(
                         NftJsonAsset::new(
+                            this.app_project.project(),
                             *rsc_chunk,
-                            this.app_project.project().output_fs(),
-                            this.app_project.project().project_fs(),
-                            this.app_project.project().client_fs(),
                             client_reference_manifest.iter().map(|m| **m).collect(),
                         )
                         .to_resolved()

crates/next-api/src/instrumentation.rs

@@ -225,15 +225,9 @@ impl InstrumentationEndpoint {
             let mut output_assets = vec![chunk];
             if this.project.next_mode().await?.is_production() {
                 output_assets.push(ResolvedVc::upcast(
-                    NftJsonAsset::new(
-                        *chunk,
-                        this.project.output_fs(),
-                        this.project.project_fs(),
-                        this.project.client_fs(),
-                        vec![],
-                    )
-                    .to_resolved()
-                    .await?,
+                    NftJsonAsset::new(this.project, *chunk, vec![])
+                        .to_resolved()
+                        .await?,
                 ));
             }
             Ok(Vc::cell(output_assets))

crates/next-api/src/nft_json.rs

@@ -1,15 +1,19 @@
+use std::collections::BTreeSet;
+
 use anyhow::{bail, Result};
 use serde_json::json;
 use turbo_rcstr::RcStr;
 use turbo_tasks::{ResolvedVc, ValueToString, Vc};
-use turbo_tasks_fs::{DiskFileSystem, File, FileSystem, FileSystemPath, VirtualFileSystem};
+use turbo_tasks_fs::{File, FileSystem, FileSystemPath};
 use turbopack_core::{
     asset::{Asset, AssetContent},
     ident::AssetIdent,
     output::OutputAsset,
     reference::all_assets_from_entries,
 };
 
+use crate::project::Project;
+
 /// A json file that produces references to all files that are needed by the given module
 /// at runtime. This will include, for example, node native modules, unanalyzable packages,
 /// client side chunks, etc.
@@ -18,11 +22,9 @@ use turbopack_core::{
 /// their bundle.
 #[turbo_tasks::value(shared)]
 pub struct NftJsonAsset {
+    project: ResolvedVc<Project>,
     /// The chunk for which the asset is being generated
-    chunk: Vc<Box<dyn OutputAsset>>,
-    output_fs: Vc<DiskFileSystem>,
-    project_fs: Vc<DiskFileSystem>,
-    client_fs: Vc<Box<dyn FileSystem>>,
+    chunk: ResolvedVc<Box<dyn OutputAsset>>,
     /// Additional assets to include in the nft json. This can be used to manually collect assets
     /// that are known to be required but are not in the graph yet, for whatever reason.
     ///
@@ -35,21 +37,44 @@ pub struct NftJsonAsset {
 impl NftJsonAsset {
     #[turbo_tasks::function]
     pub fn new(
-        chunk: Vc<Box<dyn OutputAsset>>,
-        output_fs: Vc<DiskFileSystem>,
-        project_fs: Vc<DiskFileSystem>,
-        client_fs: Vc<Box<dyn FileSystem>>,
+        project: ResolvedVc<Project>,
+        chunk: ResolvedVc<Box<dyn OutputAsset>>,
         additional_assets: Vec<ResolvedVc<Box<dyn OutputAsset>>>,
     ) -> Vc<Self> {
         NftJsonAsset {
             chunk,
-            output_fs,
-            project_fs,
-            client_fs,
+            project,
             additional_assets,
         }
         .cell()
     }
+
+    #[turbo_tasks::function]
+    fn output_root(&self) -> Vc<FileSystemPath> {
+        self.project.output_fs().root()
+    }
+
+    #[turbo_tasks::function]
+    fn project_root(&self) -> Vc<FileSystemPath> {
+        self.project.project_fs().root()
+    }
+
+    #[turbo_tasks::function]
+    fn client_root(&self) -> Vc<FileSystemPath> {
+        self.project.client_fs().root()
+    }
+
+    #[turbo_tasks::function]
+    fn project_path(&self) -> Vc<FileSystemPath> {
+        self.project.project_path()
+    }
+
+    #[turbo_tasks::function]
+    async fn dist_dir(&self) -> Result<Vc<RcStr>> {
+        Ok(Vc::cell(
+            format!("/{}/", self.project.dist_dir().await?).into(),
+        ))
+    }
 }
 
 #[turbo_tasks::value(transparent)]
@@ -58,81 +83,68 @@ pub struct OutputSpecifier(Option<RcStr>);
 #[turbo_tasks::value_impl]
 impl NftJsonAsset {
     #[turbo_tasks::function]
-    async fn ident_in_project_fs(self: Vc<Self>) -> Result<Vc<FileSystemPath>> {
-        let this = self.await?;
-        let project_fs = this.project_fs.await?;
-        let output_fs = this.output_fs.await?;
-        let nft_folder = self.ident().path().parent().await?;
-
-        if let Some(subdir) = output_fs.root().strip_prefix(&**project_fs.root()) {
-            Ok(this
-                .project_fs
-                .root()
-                .join(subdir.into())
-                .join(nft_folder.path.clone()))
-        } else {
-            // TODO: what are the implications of this?
-            bail!("output fs not inside project fs");
-        }
+    async fn ident_folder(self: Vc<Self>) -> Vc<FileSystemPath> {
+        self.ident().path().parent()
     }
 
     #[turbo_tasks::function]
-    async fn ident_in_client_fs(self: Vc<Self>) -> Result<Vc<FileSystemPath>> {
+    async fn ident_folder_in_project_fs(self: Vc<Self>) -> Result<Vc<FileSystemPath>> {
         Ok(self
-            .await?
-            .client_fs
-            .root()
-            .join(self.ident().path().parent().await?.path.clone()))
+            .project_path()
+            .join(self.ident_folder().await?.path.clone()))
+    }
+
+    #[turbo_tasks::function]
+    async fn ident_folder_in_client_fs(self: Vc<Self>) -> Result<Vc<FileSystemPath>> {
+        Ok(self
+            .client_root()
+            .join(self.ident_folder().await?.path.clone()))
     }
 
     #[turbo_tasks::function]
     async fn get_output_specifier(
         self: Vc<Self>,
         path: Vc<FileSystemPath>,
     ) -> Result<Vc<OutputSpecifier>> {
-        let this = self.await?;
-        let path_fs = path.fs().resolve().await?;
         let path_ref = path.await?;
-        let nft_folder = self.ident().path().parent().await?;
 
-        if path_fs == Vc::upcast(this.output_fs.resolve().await?) {
-            // e.g. a referenced chunk
+        // include assets in the outputs such as referenced chunks
+        if path_ref.is_inside_ref(&*(self.output_root().await?)) {
             return Ok(Vc::cell(Some(
-                nft_folder.get_relative_path_to(&path_ref).unwrap(),
+                self.ident_folder()
+                    .await?
+                    .get_relative_path_to(&path_ref)
+                    .unwrap(),
             )));
-        } else if path_fs == Vc::upcast(this.project_fs.resolve().await?) {
+        }
+
+        // include assets in the project root such as images and traced references (externals)
+        if path_ref.is_inside_ref(&*(self.project_root().await?)) {
             return Ok(Vc::cell(Some(
-                self.ident_in_project_fs()
+                self.ident_folder_in_project_fs()
                     .await?
                     .get_relative_path_to(&path_ref)
                     .unwrap(),
             )));
-        } else if path_fs == Vc::upcast(this.client_fs.resolve().await?) {
+        }
+
+        // assets that are needed on the client side such as fonts and icons
+        if path_ref.is_inside_ref(&*(self.client_root().await?)) {
             return Ok(Vc::cell(Some(
-                self.ident_in_client_fs()
+                self.ident_folder_in_client_fs()
                     .await?
                     .get_relative_path_to(&path_ref)
                     .unwrap()
-                    .replace("/_next/", "/.next/")
+                    .replace("/_next/", &self.dist_dir().await?)
                     .into(),
             )));
         }
 
-        if let Some(path_fs) = Vc::try_resolve_downcast_type::<VirtualFileSystem>(path_fs).await? {
-            if path_fs.await?.name == "externals" || path_fs.await?.name == "traced" {
-                return Ok(Vc::cell(Some(
-                    self.ident_in_project_fs()
-                        .await?
-                        .get_relative_path_to(
-                            &*this.project_fs.root().join(path_ref.path.clone()).await?,
-                        )
-                        .unwrap(),
-                )));
-            }
-        }
-
-        println!("Unknown filesystem for {}", path.to_string().await?);
-        Ok(Vc::cell(None))
+        // Make this an error for now, this should effectively be unreachable
+        bail!(
+            "NftJsonAsset: cannot handle filepath {}",
+            path.to_string().await?
+        );
     }
 }
 
@@ -154,7 +166,7 @@ impl Asset for NftJsonAsset {
     #[turbo_tasks::function]
     async fn content(self: Vc<Self>) -> Result<Vc<AssetContent>> {
         let this = &*self.await?;
-        let mut result = Vec::new();
+        let mut result = BTreeSet::new();
 
         let chunk = this.chunk.to_resolved().await?;
         let entries = this
@@ -176,12 +188,10 @@ impl Asset for NftJsonAsset {
                 .get_output_specifier(referenced_chunk.ident().path())
                 .await?;
             if let Some(specifier) = &*specifier {
-                result.push(specifier.clone());
+                result.insert(specifier.clone());
             }
         }
 
-        result.sort();
-        result.dedup();
         let json = json!({
           "version": 1,
           "files": result

crates/next-api/src/pages.rs

@@ -882,23 +882,17 @@ impl PageEndpoint {
                 )
                 .await?;
 
-                let nft = if this
+                let server_asset_trace_file = if this
                     .pages_project
                     .project()
                     .next_mode()
                     .await?
                     .is_production()
                 {
                     ResolvedVc::cell(Some(ResolvedVc::upcast(
-                        NftJsonAsset::new(
-                            *ssr_entry_chunk,
-                            this.pages_project.project().output_fs(),
-                            this.pages_project.project().project_fs(),
-                            this.pages_project.project().client_fs(),
-                            vec![],
-                        )
-                        .to_resolved()
-                        .await?,
+                        NftJsonAsset::new(this.pages_project.project(), *ssr_entry_chunk, vec![])
+                            .to_resolved()
+                            .await?,
                     )))
                 } else {
                     ResolvedVc::cell(None)
@@ -907,7 +901,7 @@ impl PageEndpoint {
                 Ok(SsrChunk::NodeJs {
                     entry: ssr_entry_chunk,
                     dynamic_import_entries,
-                    nft,
+                    server_asset_trace_file,
                 }
                 .cell())
             }
@@ -1109,11 +1103,11 @@ impl PageEndpoint {
             SsrChunk::NodeJs {
                 entry,
                 dynamic_import_entries,
-                nft,
+                server_asset_trace_file,
             } => {
                 server_assets.push(entry);
-                if let Some(nft) = &*nft.await? {
-                    server_assets.push(*nft);
+                if let Some(server_asset_trace_file) = &*server_asset_trace_file.await? {
+                    server_assets.push(*server_asset_trace_file);
                 }
 
                 if emit_manifests {
@@ -1396,7 +1390,7 @@ pub enum SsrChunk {
     NodeJs {
         entry: ResolvedVc<Box<dyn OutputAsset>>,
         dynamic_import_entries: Vc<DynamicImportedChunks>,
-        nft: ResolvedVc<OptionOutputAsset>,
+        server_asset_trace_file: ResolvedVc<OptionOutputAsset>,
     },
     Edge {
         files: Vc<OutputAssets>,

turbopack/crates/turbopack-core/src/chunk/chunk_group.rs

@@ -6,7 +6,6 @@ use futures::future::try_join_all;
 use turbo_tasks::{
     FxIndexMap, FxIndexSet, ResolvedVc, TryFlatJoinIterExt, TryJoinIterExt, Value, Vc,
 };
-use turbo_tasks_fs::{FileSystem, VirtualFileSystem};
 
 use super::{
     availability_info::AvailabilityInfo, available_chunk_items::AvailableChunkItemInfo,
@@ -154,7 +153,12 @@ pub async fn make_chunk_group(
         .clone_value();
 
     let rebased_modules = try_join_all(traced_modules.into_iter().map(|module| {
-        RebasedAsset::new(*module, module.ident().path().root(), traced_fs().root()).to_resolved()
+        RebasedAsset::new(
+            *module,
+            module.ident().path().root(),
+            module.ident().path().root(),
+        )
+        .to_resolved()
     }))
     .await?;
 
@@ -198,12 +202,6 @@ pub async fn make_chunk_group(
     })
 }
 
-// Without this wrapper, VirtualFileSystem::new_with_name always returns a new filesystem
-#[turbo_tasks::function]
-fn traced_fs() -> Vc<VirtualFileSystem> {
-    VirtualFileSystem::new_with_name("traced".into())
-}
-
 async fn references_to_output_assets(
     references: FxIndexSet<Vc<Box<dyn ModuleReference>>>,
 ) -> Result<Vc<OutputAssets>> {