Turbopack: remove OptionStringifiedSourceMap (#86547)

- `OptionStringifiedSourceMap` was always a strange type
- We can now use the `FileContent` to not clone the Rope in the `SourceMapAsset::content`, which would end up duplicating sourcemaps in the cache.

Author: mischnic

crates/napi/src/next_api/project.rs

@@ -51,7 +51,7 @@ use turbopack_core::{
     error::PrettyPrintError,
     issue::PlainIssue,
     output::{OutputAsset, OutputAssets},
-    source_map::{OptionStringifiedSourceMap, SourceMap, Token},
+    source_map::{SourceMap, Token},
     version::{PartialUpdate, TotalUpdate, Update, VersionState},
 };
 use turbopack_ecmascript_hmr_protocol::{ClientUpdateInstruction, Issue, ResourceIdentifier};
@@ -1550,7 +1550,7 @@ pub struct OptionStackFrame(Option<StackFrame>);
 pub async fn get_source_map_rope(
     container: Vc<ProjectContainer>,
     source_url: RcStr,
-) -> Result<Vc<OptionStringifiedSourceMap>> {
+) -> Result<Vc<FileContent>> {
     let (file_path_sys, module) = match Url::parse(&source_url) {
         Ok(url) => match url.scheme() {
             "file" => {
@@ -1579,7 +1579,7 @@ pub async fn get_source_map_rope(
             Some(relative_path) => sys_to_unix(relative_path),
             None => {
                 // File doesn't exist within the dist dir
-                return Ok(OptionStringifiedSourceMap::none());
+                return Ok(FileContent::NotFound.cell());
             }
         };
 
@@ -1597,13 +1597,13 @@ pub async fn get_source_map_rope(
 
     let mut map = container.get_source_map(server_path, module.clone());
 
-    if map.await?.is_none() {
+    if !map.await?.is_content() {
         // If the chunk doesn't exist as a server chunk, try a client chunk.
         // TODO: Properly tag all server chunks and use the `isServer` query param.
         // Currently, this is inaccurate as it does not cover RSC server
         // chunks.
         map = container.get_source_map(client_path, module);
-        if map.await?.is_none() {
+        if !map.await?.is_content() {
             bail!("chunk/module '{}' is missing a sourcemap", source_url);
         }
     }
@@ -1615,7 +1615,7 @@ pub async fn get_source_map_rope(
 pub fn get_source_map_rope_operation(
     container: ResolvedVc<ProjectContainer>,
     file_path: RcStr,
-) -> Vc<OptionStringifiedSourceMap> {
+) -> Vc<FileContent> {
     get_source_map_rope(*container, file_path)
 }
 
@@ -1782,13 +1782,13 @@ pub async fn project_get_source_map(
     let ctx = &project.turbopack_ctx;
     ctx.turbo_tasks()
         .run(async move {
-            let Some(map) = &*get_source_map_rope_operation(container, file_path)
+            let source_map = get_source_map_rope_operation(container, file_path)
                 .read_strongly_consistent()
-                .await?
-            else {
+                .await?;
+            let Some(map) = source_map.as_content() else {
                 return Ok(None);
             };
-            Ok(Some(map.to_str()?.to_string()))
+            Ok(Some(map.content().to_str()?.to_string()))
         })
         // HACK: Don't use `TurbopackInternalError`, this function is race-condition prone (the
         // source files may have changed or been deleted), so these probably aren't internal errors?

crates/next-api/src/project.rs

@@ -33,7 +33,9 @@ use turbo_tasks::{
     debug::ValueDebugFormat, fxindexmap, mark_root, trace::TraceRawVcs,
 };
 use turbo_tasks_env::{EnvMap, ProcessEnv};
-use turbo_tasks_fs::{DiskFileSystem, FileSystem, FileSystemPath, VirtualFileSystem, invalidation};
+use turbo_tasks_fs::{
+    DiskFileSystem, FileContent, FileSystem, FileSystemPath, VirtualFileSystem, invalidation,
+};
 use turbo_unix_path::{join_path, unix_to_sys};
 use turbopack::{
     ModuleAssetContext, evaluate_context::node_build_environment,
@@ -70,7 +72,6 @@ use turbopack_core::{
     },
     reference::all_assets_from_entries,
     resolve::{FindContextFileResult, find_context_file},
-    source_map::OptionStringifiedSourceMap,
     version::{
         NotFoundVersion, OptionVersionedContent, Update, Version, VersionState, VersionedContent,
     },
@@ -552,17 +553,17 @@ impl ProjectContainer {
     }
 
     /// Gets a source map for a particular `file_path`. If `dev` mode is disabled, this will always
-    /// return [`OptionStringifiedSourceMap::none`].
+    /// return [`FileContent::NotFound`].
     #[turbo_tasks::function]
     pub fn get_source_map(
         &self,
         file_path: FileSystemPath,
         section: Option<RcStr>,
-    ) -> Vc<OptionStringifiedSourceMap> {
+    ) -> Vc<FileContent> {
         if let Some(map) = self.versioned_content_map {
             map.get_source_map(file_path, section)
         } else {
-            OptionStringifiedSourceMap::none()
+            FileContent::NotFound.cell()
         }
     }
 }

crates/next-api/src/versioned_content_map.rs

@@ -7,11 +7,11 @@ use turbo_tasks::{
     FxIndexSet, NonLocalValue, OperationValue, OperationVc, ResolvedVc, State, TryFlatJoinIterExt,
     TryJoinIterExt, ValueDefault, Vc, debug::ValueDebugFormat, trace::TraceRawVcs,
 };
-use turbo_tasks_fs::FileSystemPath;
+use turbo_tasks_fs::{FileContent, FileSystemPath};
 use turbopack_core::{
     asset::Asset,
     output::{ExpandedOutputAssets, OptionOutputAsset, OutputAsset},
-    source_map::{GenerateSourceMap, OptionStringifiedSourceMap},
+    source_map::GenerateSourceMap,
     version::OptionVersionedContent,
 };
 
@@ -180,9 +180,9 @@ impl VersionedContentMap {
         self: Vc<Self>,
         path: FileSystemPath,
         section: Option<RcStr>,
-    ) -> Result<Vc<OptionStringifiedSourceMap>> {
+    ) -> Result<Vc<FileContent>> {
         let Some(asset) = &*self.get_asset(path.clone()).await? else {
-            return Ok(Vc::cell(None));
+            return Ok(FileContent::NotFound.cell());
         };
 
         if let Some(generate_source_map) =

crates/next-core/src/raw_ecmascript_module.rs

@@ -272,7 +272,8 @@ impl EcmascriptChunkItem for RawEcmascriptChunkItem {
             )
             .await?
             {
-                source_map.generate_source_map().owned().await?
+                let source_map = source_map.generate_source_map().await?;
+                source_map.as_content().map(|f| f.content().clone())
             } else {
                 None
             };

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

@@ -1766,6 +1766,7 @@ impl From<File> for FileContent {
     }
 }
 
+// TODO remove this, it hides cell creation
 impl From<File> for Vc<FileContent> {
     fn from(file: File) -> Self {
         FileContent::Content(file).cell()

turbopack/crates/turbopack-analyze/src/split_chunk.rs

@@ -47,10 +47,11 @@ pub async fn split_output_asset_into_parts(
     else {
         return self_mapped(asset, content).await;
     };
-    let Some(source_map) = &*generate_source_map.generate_source_map().await? else {
+    let source_map = generate_source_map.generate_source_map().await?;
+    let Some(source_map) = source_map.as_content() else {
         return self_mapped(asset, content).await;
     };
-    let Some(source_map) = SourceMap::new_from_rope(source_map)? else {
+    let Some(source_map) = SourceMap::new_from_rope(source_map.content())? else {
         return unaccounted(asset, content).await;
     };
 

turbopack/crates/turbopack-analyze/tests/split_chunk.rs

@@ -6,14 +6,16 @@ use anyhow::Result;
 use serde_json::json;
 use turbo_rcstr::rcstr;
 use turbo_tasks::{ResolvedVc, Vc};
-use turbo_tasks_fs::{File, FileSystem, FileSystemPath, VirtualFileSystem, rope::Rope};
+use turbo_tasks_fs::{
+    File, FileContent, FileSystem, FileSystemPath, VirtualFileSystem, rope::Rope,
+};
 use turbo_tasks_testing::{Registration, register, run_once};
 use turbopack_analyze::split_chunk::{ChunkPart, ChunkPartRange, split_output_asset_into_parts};
 use turbopack_core::{
     asset::{Asset, AssetContent},
     code_builder::{Code, CodeBuilder},
     output::{OutputAsset, OutputAssetsReference},
-    source_map::{GenerateSourceMap, OptionStringifiedSourceMap},
+    source_map::GenerateSourceMap,
 };
 
 static REGISTRATION: Registration = register!(turbo_tasks_fetch::register);
@@ -130,7 +132,7 @@ impl Asset for TestAsset {
 #[turbo_tasks::value_impl]
 impl GenerateSourceMap for TestAsset {
     #[turbo_tasks::function]
-    pub fn generate_source_map(&self) -> Vc<OptionStringifiedSourceMap> {
+    pub fn generate_source_map(&self) -> Vc<FileContent> {
         self.code.generate_source_map()
     }
 }

turbopack/crates/turbopack-browser/src/ecmascript/chunk.rs

@@ -1,14 +1,14 @@
 use anyhow::Result;
 use turbo_rcstr::{RcStr, rcstr};
 use turbo_tasks::{FxIndexSet, ResolvedVc, ValueToString, Vc};
-use turbo_tasks_fs::FileSystemPath;
+use turbo_tasks_fs::{FileContent, FileSystemPath};
 use turbopack_core::{
     asset::{Asset, AssetContent},
     chunk::{Chunk, ChunkingContext, OutputChunk, OutputChunkRuntimeInfo},
     ident::AssetIdent,
     introspect::{Introspectable, IntrospectableChildren},
     output::{OutputAsset, OutputAssetsReference, OutputAssetsWithReferenced},
-    source_map::{GenerateSourceMap, OptionStringifiedSourceMap, SourceMapAsset},
+    source_map::{GenerateSourceMap, SourceMapAsset},
     version::VersionedContent,
 };
 use turbopack_ecmascript::chunk::EcmascriptChunk;
@@ -157,12 +157,12 @@ impl Asset for EcmascriptBrowserChunk {
 #[turbo_tasks::value_impl]
 impl GenerateSourceMap for EcmascriptBrowserChunk {
     #[turbo_tasks::function]
-    fn generate_source_map(self: Vc<Self>) -> Vc<OptionStringifiedSourceMap> {
+    fn generate_source_map(self: Vc<Self>) -> Vc<FileContent> {
         self.own_content().generate_source_map()
     }
 
     #[turbo_tasks::function]
-    fn by_section(self: Vc<Self>, section: RcStr) -> Vc<OptionStringifiedSourceMap> {
+    fn by_section(self: Vc<Self>, section: RcStr) -> Vc<FileContent> {
         self.own_content().by_section(section)
     }
 }

turbopack/crates/turbopack-browser/src/ecmascript/content.rs

@@ -4,13 +4,13 @@ use anyhow::{Result, bail};
 use either::Either;
 use turbo_rcstr::RcStr;
 use turbo_tasks::{ResolvedVc, Vc};
-use turbo_tasks_fs::File;
+use turbo_tasks_fs::{File, FileContent};
 use turbopack_core::{
     asset::AssetContent,
     chunk::{ChunkingContext, MinifyType, ModuleId},
     code_builder::{Code, CodeBuilder},
     output::OutputAsset,
-    source_map::{GenerateSourceMap, OptionStringifiedSourceMap, SourceMapAsset},
+    source_map::{GenerateSourceMap, SourceMapAsset},
     version::{MergeableVersionedContent, Version, VersionedContent, VersionedContentMerger},
 };
 use turbopack_ecmascript::{chunk::EcmascriptChunkContent, minify::minify, utils::StringifyJs};
@@ -166,12 +166,12 @@ impl MergeableVersionedContent for EcmascriptBrowserChunkContent {
 #[turbo_tasks::value_impl]
 impl GenerateSourceMap for EcmascriptBrowserChunkContent {
     #[turbo_tasks::function]
-    fn generate_source_map(self: Vc<Self>) -> Vc<OptionStringifiedSourceMap> {
+    fn generate_source_map(self: Vc<Self>) -> Vc<FileContent> {
         self.code().generate_source_map()
     }
 
     #[turbo_tasks::function]
-    async fn by_section(self: Vc<Self>, section: RcStr) -> Result<Vc<OptionStringifiedSourceMap>> {
+    async fn by_section(self: Vc<Self>, section: RcStr) -> Result<Vc<FileContent>> {
         // Weirdly, the ContentSource will have already URL decoded the ModuleId, and we
         // can't reparse that via serde.
         if let Ok(id) = ModuleId::parse(&section) {
@@ -184,6 +184,6 @@ impl GenerateSourceMap for EcmascriptBrowserChunkContent {
             }
         }
 
-        Ok(Vc::cell(None))
+        Ok(FileContent::NotFound.cell())
     }
 }

turbopack/crates/turbopack-browser/src/ecmascript/evaluate/chunk.rs

@@ -6,7 +6,7 @@ use indoc::writedoc;
 use serde::Serialize;
 use turbo_rcstr::{RcStr, rcstr};
 use turbo_tasks::{ReadRef, ResolvedVc, TryJoinIterExt, ValueToString, Vc};
-use turbo_tasks_fs::{File, FileSystemPath};
+use turbo_tasks_fs::{File, FileContent, FileSystemPath};
 use turbopack_core::{
     asset::{Asset, AssetContent},
     chunk::{
@@ -18,7 +18,7 @@ use turbopack_core::{
     module::Module,
     module_graph::ModuleGraph,
     output::{OutputAsset, OutputAssets, OutputAssetsReference, OutputAssetsWithReferenced},
-    source_map::{GenerateSourceMap, OptionStringifiedSourceMap, SourceMapAsset},
+    source_map::{GenerateSourceMap, SourceMapAsset},
 };
 use turbopack_ecmascript::{
     chunk::{EcmascriptChunkData, EcmascriptChunkPlaceable},
@@ -299,7 +299,7 @@ impl Asset for EcmascriptBrowserEvaluateChunk {
 #[turbo_tasks::value_impl]
 impl GenerateSourceMap for EcmascriptBrowserEvaluateChunk {
     #[turbo_tasks::function]
-    fn generate_source_map(self: Vc<Self>) -> Vc<OptionStringifiedSourceMap> {
+    fn generate_source_map(self: Vc<Self>) -> Vc<FileContent> {
         self.code().generate_source_map()
     }
 }