refactor(turbopack/next-api): Make VcArc use OperationVc (#74479)

`VcArc` represents values passed through napi to next.js and back. Because these values exit the scope of turbo-tasks, similar to how `State` works, they must be operations, so that they're recomputed/"kept alive" when read.

These changes were extracted from @sokra's original `OperationVc` work (mostly preserved here: #72776), though adapted significantly for the removal of the `OperationVc::new()` constructor.

Author: bgw

crates/napi/src/next_api/endpoint.rs

@@ -10,9 +10,7 @@ use next_api::{
     },
 };
 use tracing::Instrument;
-use turbo_tasks::{
-    get_effects, Completion, Effects, OperationVc, ReadRef, ResolvedVc, Vc, VcValueType,
-};
+use turbo_tasks::{get_effects, Completion, Effects, OperationVc, ReadRef, Vc, VcValueType};
 use turbopack_core::{
     diagnostics::PlainDiagnostic,
     error::PrettyPrintError,
@@ -91,10 +89,10 @@ impl From<Option<WrittenEndpoint>> for NapiWrittenEndpoint {
 //    some async functions (in this case `endpoint_write_to_disk`) can cause
 //    higher-ranked lifetime errors. See https://github.com/rust-lang/rust/issues/102211
 // 2. the type_complexity clippy lint.
-pub struct ExternalEndpoint(pub VcArc<Vc<Box<dyn Endpoint>>>);
+pub struct ExternalEndpoint(pub VcArc<Box<dyn Endpoint>>);
 
 impl Deref for ExternalEndpoint {
-    type Target = VcArc<Vc<Box<dyn Endpoint>>>;
+    type Target = VcArc<Box<dyn Endpoint>>;
 
     fn deref(&self) -> &Self::Target {
         &self.0
@@ -135,9 +133,9 @@ struct WrittenEndpointWithIssues {
 
 #[turbo_tasks::function]
 async fn get_written_endpoint_with_issues(
-    endpoint: ResolvedVc<Box<dyn Endpoint>>,
+    endpoint_op: OperationVc<Box<dyn Endpoint>>,
 ) -> Result<Vc<WrittenEndpointWithIssues>> {
-    let write_to_disk = endpoint_write_to_disk_operation(endpoint);
+    let write_to_disk = endpoint_write_to_disk_operation(endpoint_op);
     let (written, issues, diagnostics, effects) =
         strongly_consistent_catch_collectables(write_to_disk).await?;
     Ok(WrittenEndpointWithIssues {
@@ -155,10 +153,10 @@ pub async fn endpoint_write_to_disk(
     #[napi(ts_arg_type = "{ __napiType: \"Endpoint\" }")] endpoint: External<ExternalEndpoint>,
 ) -> napi::Result<TurbopackResult<NapiWrittenEndpoint>> {
     let turbo_tasks = endpoint.turbo_tasks().clone();
-    let endpoint = ***endpoint;
+    let endpoint_op = ***endpoint;
     let (written, issues, diags) = turbo_tasks
         .run_once(async move {
-            let operation = get_written_endpoint_with_issues(endpoint);
+            let operation = get_written_endpoint_with_issues(endpoint_op);
             let WrittenEndpointWithIssues {
                 written,
                 issues,
@@ -191,8 +189,8 @@ pub fn endpoint_server_changed_subscribe(
         func,
         move || {
             async move {
-                let operation = subscribe_issues_and_diags(endpoint, issues);
-                let result = operation.strongly_consistent().await?;
+                let vc = subscribe_issues_and_diags(endpoint, issues);
+                let result = vc.strongly_consistent().await?;
                 result.effects.apply().await?;
                 Ok(result)
             }
@@ -241,10 +239,10 @@ impl Eq for EndpointIssuesAndDiags {}
 
 #[turbo_tasks::function]
 async fn subscribe_issues_and_diags(
-    endpoint: ResolvedVc<Box<dyn Endpoint>>,
+    endpoint_op: OperationVc<Box<dyn Endpoint>>,
     should_include_issues: bool,
 ) -> Result<Vc<EndpointIssuesAndDiags>> {
-    let changed = endpoint_server_changed_operation(endpoint);
+    let changed = endpoint_server_changed_operation(endpoint_op);
 
     if should_include_issues {
         let (changed_value, issues, diagnostics, effects) =
@@ -280,7 +278,7 @@ pub fn endpoint_client_changed_subscribe(
         func,
         move || {
             async move {
-                let changed = endpoint.client_changed();
+                let changed = endpoint.connect().client_changed();
                 // We don't capture issues and diagnostics here since we don't want to be
                 // notified when they change
                 changed.strongly_consistent().await?;

crates/napi/src/next_api/project.rs

@@ -8,11 +8,14 @@ use napi::{
 };
 use next_api::{
     entrypoints::Entrypoints,
+    operation::{
+        EntrypointsOperation, InstrumentationOperation, MiddlewareOperation, RouteOperation,
+    },
     project::{
-        DefineEnv, DraftModeOptions, Instrumentation, Middleware, PartialProjectOptions, Project,
-        ProjectContainer, ProjectOptions, WatchOptions,
+        DefineEnv, DraftModeOptions, PartialProjectOptions, Project, ProjectContainer,
+        ProjectOptions, WatchOptions,
     },
-    route::{Endpoint, Route},
+    route::Endpoint,
 };
 use next_core::tracing_presets::{
     TRACING_NEXT_OVERVIEW_TARGETS, TRACING_NEXT_TARGETS, TRACING_NEXT_TURBOPACK_TARGETS,
@@ -25,7 +28,8 @@ use tracing::Instrument;
 use tracing_subscriber::{layer::SubscriberExt, util::SubscriberInitExt, Registry};
 use turbo_rcstr::RcStr;
 use turbo_tasks::{
-    get_effects, Completion, Effects, ReadRef, ResolvedVc, TransientInstance, UpdateInfo, Vc,
+    get_effects, Completion, Effects, OperationVc, ReadRef, ResolvedVc, TransientInstance,
+    UpdateInfo, Vc,
 };
 use turbo_tasks_fs::{
     get_relative_path_to, util::uri_from_file, DiskFileSystem, FileContent, FileSystem,
@@ -539,56 +543,56 @@ struct NapiRoute {
 }
 
 impl NapiRoute {
-    fn from_route(pathname: String, value: Route, turbo_tasks: &NextTurboTasks) -> Self {
-        let convert_endpoint = |endpoint: Vc<Box<dyn Endpoint>>| {
+    fn from_route(pathname: String, value: RouteOperation, turbo_tasks: &NextTurboTasks) -> Self {
+        let convert_endpoint = |endpoint: OperationVc<Box<dyn Endpoint>>| {
             Some(External::new(ExternalEndpoint(VcArc::new(
                 turbo_tasks.clone(),
                 endpoint,
             ))))
         };
         match value {
-            Route::Page {
+            RouteOperation::Page {
                 html_endpoint,
                 data_endpoint,
             } => NapiRoute {
                 pathname,
                 r#type: "page",
-                html_endpoint: convert_endpoint(*html_endpoint),
-                data_endpoint: convert_endpoint(*data_endpoint),
+                html_endpoint: convert_endpoint(html_endpoint),
+                data_endpoint: convert_endpoint(data_endpoint),
                 ..Default::default()
             },
-            Route::PageApi { endpoint } => NapiRoute {
+            RouteOperation::PageApi { endpoint } => NapiRoute {
                 pathname,
                 r#type: "page-api",
-                endpoint: convert_endpoint(*endpoint),
+                endpoint: convert_endpoint(endpoint),
                 ..Default::default()
             },
-            Route::AppPage(pages) => NapiRoute {
+            RouteOperation::AppPage(pages) => NapiRoute {
                 pathname,
                 r#type: "app-page",
                 pages: Some(
                     pages
                         .into_iter()
                         .map(|page_route| AppPageNapiRoute {
-                            original_name: Some(page_route.original_name),
+                            original_name: Some(page_route.original_name.into_owned()),
                             html_endpoint: convert_endpoint(page_route.html_endpoint),
                             rsc_endpoint: convert_endpoint(page_route.rsc_endpoint),
                         })
                         .collect(),
                 ),
                 ..Default::default()
             },
-            Route::AppRoute {
+            RouteOperation::AppRoute {
                 original_name,
                 endpoint,
             } => NapiRoute {
                 pathname,
-                original_name: Some(original_name),
+                original_name: Some(original_name.into_owned()),
                 r#type: "app-route",
-                endpoint: convert_endpoint(*endpoint),
+                endpoint: convert_endpoint(endpoint),
                 ..Default::default()
             },
-            Route::Conflict => NapiRoute {
+            RouteOperation::Conflict => NapiRoute {
                 pathname,
                 r#type: "conflict",
                 ..Default::default()
@@ -603,7 +607,7 @@ struct NapiMiddleware {
 }
 
 impl NapiMiddleware {
-    fn from_middleware(value: &Middleware, turbo_tasks: &NextTurboTasks) -> Result<Self> {
+    fn from_middleware(value: &MiddlewareOperation, turbo_tasks: &NextTurboTasks) -> Result<Self> {
         Ok(NapiMiddleware {
             endpoint: External::new(ExternalEndpoint(VcArc::new(
                 turbo_tasks.clone(),
@@ -620,7 +624,10 @@ struct NapiInstrumentation {
 }
 
 impl NapiInstrumentation {
-    fn from_instrumentation(value: &Instrumentation, turbo_tasks: &NextTurboTasks) -> Result<Self> {
+    fn from_instrumentation(
+        value: &InstrumentationOperation,
+        turbo_tasks: &NextTurboTasks,
+    ) -> Result<Self> {
         Ok(NapiInstrumentation {
             node_js: External::new(ExternalEndpoint(VcArc::new(
                 turbo_tasks.clone(),
@@ -644,9 +651,9 @@ struct NapiEntrypoints {
     pub pages_error_endpoint: External<ExternalEndpoint>,
 }
 
-#[turbo_tasks::value(serialization = "none", local)]
+#[turbo_tasks::value(serialization = "none")]
 struct EntrypointsWithIssues {
-    entrypoints: ReadRef<Entrypoints>,
+    entrypoints: ReadRef<EntrypointsOperation>,
     issues: Arc<Vec<ReadRef<PlainIssue>>>,
     diagnostics: Arc<Vec<ReadRef<PlainDiagnostic>>>,
     effects: Arc<Effects>,
@@ -656,7 +663,8 @@ struct EntrypointsWithIssues {
 async fn get_entrypoints_with_issues(
     container: ResolvedVc<ProjectContainer>,
 ) -> Result<Vc<EntrypointsWithIssues>> {
-    let entrypoints_operation = project_container_entrypoints_operation(container);
+    let entrypoints_operation =
+        EntrypointsOperation::new(project_container_entrypoints_operation(container));
     let entrypoints = entrypoints_operation
         .connect()
         .strongly_consistent()
@@ -675,6 +683,8 @@ async fn get_entrypoints_with_issues(
 
 #[turbo_tasks::function(operation)]
 fn project_container_entrypoints_operation(
+    // the container is a long-lived object with internally mutable state, there's no risk of it
+    // becoming stale
     container: ResolvedVc<ProjectContainer>,
 ) -> Vc<Entrypoints> {
     container.entrypoints()
@@ -732,15 +742,15 @@ pub fn project_entrypoints_subscribe(
                         .transpose()?,
                     pages_document_endpoint: External::new(ExternalEndpoint(VcArc::new(
                         turbo_tasks.clone(),
-                        *entrypoints.pages_document_endpoint,
+                        entrypoints.pages_document_endpoint,
                     ))),
                     pages_app_endpoint: External::new(ExternalEndpoint(VcArc::new(
                         turbo_tasks.clone(),
-                        *entrypoints.pages_app_endpoint,
+                        entrypoints.pages_app_endpoint,
                     ))),
                     pages_error_endpoint: External::new(ExternalEndpoint(VcArc::new(
                         turbo_tasks.clone(),
-                        *entrypoints.pages_error_endpoint,
+                        entrypoints.pages_error_endpoint,
                     ))),
                 },
                 issues: issues
@@ -782,7 +792,7 @@ async fn hmr_update(
 }
 
 #[turbo_tasks::function(operation)]
-async fn project_hmr_update_operation(
+fn project_hmr_update_operation(
     project: ResolvedVc<Project>,
     identifier: RcStr,
     state: ResolvedVc<VersionState>,

crates/napi/src/next_api/utils.rs

@@ -175,13 +175,12 @@ pub fn create_turbo_tasks(
 #[derive(Clone)]
 pub struct VcArc<T> {
     turbo_tasks: NextTurboTasks,
-    /// The Vc. Must be resolved, otherwise you are referencing an inactive
-    /// operation.
-    vc: T,
+    /// The Vc. Must be unresolved, otherwise you are referencing an inactive operation.
+    vc: OperationVc<T>,
 }
 
 impl<T> VcArc<T> {
-    pub fn new(turbo_tasks: NextTurboTasks, vc: T) -> Self {
+    pub fn new(turbo_tasks: NextTurboTasks, vc: OperationVc<T>) -> Self {
         Self { turbo_tasks, vc }
     }
 
@@ -191,7 +190,7 @@ impl<T> VcArc<T> {
 }
 
 impl<T> Deref for VcArc<T> {
-    type Target = T;
+    type Target = OperationVc<T>;
 
     fn deref(&self) -> &Self::Target {
         &self.vc

crates/next-api/src/app.rs

@@ -804,8 +804,8 @@ pub fn app_entry_point_to_route(
             pages
                 .into_iter()
                 .map(|page| AppPageRoute {
-                    original_name: page.to_string(),
-                    html_endpoint: Vc::upcast(
+                    original_name: RcStr::from(page.to_string()),
+                    html_endpoint: ResolvedVc::upcast(
                         AppEndpoint {
                             ty: AppEndpointType::Page {
                                 ty: AppPageEndpointType::Html,
@@ -814,9 +814,9 @@ pub fn app_entry_point_to_route(
                             app_project,
                             page: page.clone(),
                         }
-                        .cell(),
+                        .resolved_cell(),
                     ),
-                    rsc_endpoint: Vc::upcast(
+                    rsc_endpoint: ResolvedVc::upcast(
                         AppEndpoint {
                             ty: AppEndpointType::Page {
                                 ty: AppPageEndpointType::Rsc,
@@ -825,7 +825,7 @@ pub fn app_entry_point_to_route(
                             app_project,
                             page,
                         }
-                        .cell(),
+                        .resolved_cell(),
                     ),
                 })
                 .collect(),
@@ -835,7 +835,7 @@ pub fn app_entry_point_to_route(
             path,
             root_layouts,
         } => Route::AppRoute {
-            original_name: page.to_string(),
+            original_name: page.to_string().into(),
             endpoint: ResolvedVc::upcast(
                 AppEndpoint {
                     ty: AppEndpointType::Route { path, root_layouts },
@@ -846,7 +846,7 @@ pub fn app_entry_point_to_route(
             ),
         },
         AppEntrypoint::AppMetadata { page, metadata } => Route::AppRoute {
-            original_name: page.to_string(),
+            original_name: page.to_string().into(),
             endpoint: ResolvedVc::upcast(
                 AppEndpoint {
                     ty: AppEndpointType::Metadata { metadata },

crates/next-api/src/entrypoints.rs

@@ -6,7 +6,7 @@ use crate::{
     route::{Endpoint, Route},
 };
 
-#[turbo_tasks::value(shared, local)]
+#[turbo_tasks::value(shared)]
 pub struct Entrypoints {
     pub routes: FxIndexMap<RcStr, Route>,
     pub middleware: Option<Middleware>,

crates/next-api/src/global_module_id_strategy.rs

@@ -31,14 +31,12 @@ impl GlobalModuleIdStrategyBuilder {
         preprocessed_module_ids.push(preprocess_module_ids(*entrypoints.pages_document_endpoint));
 
         if let Some(middleware) = &entrypoints.middleware {
-            preprocessed_module_ids.push(preprocess_module_ids(middleware.endpoint));
+            preprocessed_module_ids.push(preprocess_module_ids(*middleware.endpoint));
         }
 
         if let Some(instrumentation) = &entrypoints.instrumentation {
-            let node_js = instrumentation.node_js;
-            let edge = instrumentation.edge;
-            preprocessed_module_ids.push(preprocess_module_ids(node_js));
-            preprocessed_module_ids.push(preprocess_module_ids(edge));
+            preprocessed_module_ids.push(preprocess_module_ids(*instrumentation.node_js));
+            preprocessed_module_ids.push(preprocess_module_ids(*instrumentation.edge));
         }
 
         for (_, route) in entrypoints.routes.iter() {
@@ -56,9 +54,9 @@ impl GlobalModuleIdStrategyBuilder {
                 Route::AppPage(page_routes) => {
                     for page_route in page_routes {
                         preprocessed_module_ids
-                            .push(preprocess_module_ids(page_route.html_endpoint));
+                            .push(preprocess_module_ids(*page_route.html_endpoint));
                         preprocessed_module_ids
-                            .push(preprocess_module_ids(page_route.rsc_endpoint));
+                            .push(preprocess_module_ids(*page_route.rsc_endpoint));
                     }
                 }
                 Route::AppRoute {

crates/next-api/src/lib.rs

@@ -15,6 +15,7 @@ mod loadable_manifest;
 mod middleware;
 mod module_graph;
 mod nft_json;
+pub mod operation;
 mod pages;
 pub mod paths;
 pub mod project;