From c6fe1207d6d57ea00ce2882c5af73e7daeb79a7e Mon Sep 17 00:00:00 2001 From: Benjamin Woodruff Date: Thu, 19 Dec 2024 17:39:54 -0800 Subject: [PATCH] Updates to comments --- crates/next-api/src/versioned_content_map.rs | 1 - .../turbopack-browser/src/ecmascript/list/version.rs | 4 ++++ turbopack/crates/turbopack-core/src/version.rs | 11 ++++++++--- 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/crates/next-api/src/versioned_content_map.rs b/crates/next-api/src/versioned_content_map.rs index 4b8b5ca55b4157..40cd7e42038ef4 100644 --- a/crates/next-api/src/versioned_content_map.rs +++ b/crates/next-api/src/versioned_content_map.rs @@ -29,7 +29,6 @@ use turbopack_core::{ NonLocalValue, )] struct MapEntry { - // must not be resolved assets_operation: OperationVc, /// Precomputed map for quick access to output asset by filepath path_to_asset: HashMap, ResolvedVc>>, diff --git a/turbopack/crates/turbopack-browser/src/ecmascript/list/version.rs b/turbopack/crates/turbopack-browser/src/ecmascript/list/version.rs index 7d72fe1f3a13c2..41ca7b2cd6bcb4 100644 --- a/turbopack/crates/turbopack-browser/src/ecmascript/list/version.rs +++ b/turbopack/crates/turbopack-browser/src/ecmascript/list/version.rs @@ -15,6 +15,10 @@ pub(super) struct EcmascriptDevChunkListVersion { #[turbo_tasks(trace_ignore)] pub by_path: FxIndexMap, /// A map from chunk merger to the version of the merged contents of chunks. + // + // TODO: This trace_ignore is *very* wrong, and could cause problems if/when we add a GC! + // Version is also expected not to contain `Vc`/`ResolvedVc`/`OperationVc`, and + // `turbopack_core::version::TotalUpdate` assumes it doesn't. #[turbo_tasks(trace_ignore)] pub by_merger: FxIndexMap>, VersionTraitRef>, } diff --git a/turbopack/crates/turbopack-core/src/version.rs b/turbopack/crates/turbopack-core/src/version.rs index c288616638a875..acdef54380aea9 100644 --- a/turbopack/crates/turbopack-core/src/version.rs +++ b/turbopack/crates/turbopack-core/src/version.rs @@ -119,8 +119,10 @@ impl VersionedContentExt for AssetContent { } } -/// Describes the current version of an object, and how to update them from an -/// earlier version. +/// Describes the current version of an object, and how to update them from an earlier version. +/// +/// **Important:** Implementations must not contain instances of [`Vc`]! This should describe a +/// specific version, and the value of a [`Vc`] can change due to invalidations or cache eviction. #[turbo_tasks::value_trait] pub trait Version { /// Get a unique identifier of the version as a string. There is no way @@ -191,7 +193,10 @@ pub enum Update { #[derive(PartialEq, Eq, Debug, Clone, TraceRawVcs, ValueDebugFormat, NonLocalValue)] pub struct TotalUpdate { /// The version this update will bring the object to. - // TODO: This trace_ignore is *very* wrong, and could cause problems if/when we add a GC + // + // TODO: This trace_ignore is wrong, and could cause problems if/when we add a GC. While + // `Version` assumes the implementation does not contain `Vc`, `EcmascriptDevChunkListVersion` + // is broken and violates this assumption. #[turbo_tasks(trace_ignore)] pub to: TraitRef>, }