Improve consistency of issues and diagnostics for napi calls (vercel#…

…60198)

### What?

get value, issues and diagnostics in a single strongly consistent call

### Why?

Before issues and diagnostics where not received strongly consistent,
which could result in stale data.

### How?



Closes PACK-2194

packages/next-swc/crates/napi/src/next_api/endpoint.rs

@@ -1,13 +1,16 @@
-use std::ops::Deref;
+use std::{ops::Deref, sync::Arc};
 
+use anyhow::Result;
 use napi::{bindgen_prelude::External, JsFunction};
 use next_api::{
     route::{Endpoint, WrittenEndpoint},
     server_paths::ServerPath,
 };
 use tracing::Instrument;
-use turbo_tasks::Vc;
-use turbopack_binding::turbopack::core::error::PrettyPrintError;
+use turbo_tasks::{ReadRef, Vc};
+use turbopack_binding::turbopack::core::{
+    diagnostics::PlainDiagnostic, error::PrettyPrintError, issue::PlainIssue,
+};
 
 use super::utils::{
     get_diagnostics, get_issues, subscribe, NapiDiagnostic, NapiIssue, RootTask, TurbopackResult,
@@ -80,23 +83,49 @@ impl Deref for ExternalEndpoint {
     }
 }
 
+#[turbo_tasks::value(serialization = "none")]
+struct WrittenEndpointWithIssues {
+    written: ReadRef<WrittenEndpoint>,
+    issues: Arc<Vec<ReadRef<PlainIssue>>>,
+    diagnostics: Arc<Vec<ReadRef<PlainDiagnostic>>>,
+}
+
+#[turbo_tasks::function]
+async fn get_written_endpoint_with_issues(
+    endpoint: Vc<Box<dyn Endpoint>>,
+) -> Result<Vc<WrittenEndpointWithIssues>> {
+    let write_to_disk = endpoint.write_to_disk();
+    let written = write_to_disk.strongly_consistent().await?;
+    let issues = get_issues(write_to_disk).await?;
+    let diagnostics = get_diagnostics(write_to_disk).await?;
+    Ok(WrittenEndpointWithIssues {
+        written,
+        issues,
+        diagnostics,
+    }
+    .cell())
+}
+
 #[napi]
+#[tracing::instrument(skip_all)]
 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 (written, issues, diags) = turbo_tasks
         .run_once(async move {
-            let write_to_disk = endpoint.write_to_disk();
-            let written = write_to_disk.strongly_consistent().await?;
-            let issues = get_issues(write_to_disk).await?;
-            let diags = get_diagnostics(write_to_disk).await?;
-            Ok((written, issues, diags))
+            let WrittenEndpointWithIssues {
+                written,
+                issues,
+                diagnostics,
+            } = &*get_written_endpoint_with_issues(endpoint)
+                .strongly_consistent()
+                .await?;
+            Ok((written.clone(), issues.clone(), diagnostics.clone()))
         })
         .await
         .map_err(|e| napi::Error::from_reason(PrettyPrintError(&e).to_string()))?;
-    // TODO diagnostics
     Ok(TurbopackResult {
         result: NapiWrittenEndpoint::from(&*written),
         issues: issues.iter().map(|i| NapiIssue::from(&**i)).collect(),
@@ -124,7 +153,7 @@ pub fn endpoint_server_changed_subscribe(
                     let diags = get_diagnostics(changed).await?;
                     Ok((issues, diags))
                 } else {
-                    Ok((vec![], vec![]))
+                    Ok((Arc::new(vec![]), Arc::new(vec![])))
                 }
             }
             .instrument(tracing::info_span!("server changes subscription"))

packages/next-swc/crates/napi/src/next_api/project.rs

@@ -7,8 +7,9 @@ use napi::{
     JsFunction, Status,
 };
 use next_api::{
+    entrypoints::Entrypoints,
     project::{
-        DefineEnv, Instrumentation, Middleware, PartialProjectOptions, ProjectContainer,
+        DefineEnv, Instrumentation, Middleware, PartialProjectOptions, Project, ProjectContainer,
         ProjectOptions,
     },
     route::{Endpoint, Route},
@@ -21,17 +22,19 @@ use tracing::Instrument;
 use tracing_subscriber::{
     prelude::__tracing_subscriber_SubscriberExt, util::SubscriberInitExt, EnvFilter, Registry,
 };
-use turbo_tasks::{TransientInstance, TurboTasks, UpdateInfo, Vc};
+use turbo_tasks::{ReadRef, TransientInstance, TurboTasks, UpdateInfo, Vc};
 use turbopack_binding::{
     turbo::{
         tasks_fs::{FileContent, FileSystem},
         tasks_memory::MemoryBackend,
     },
     turbopack::{
         core::{
+            diagnostics::PlainDiagnostic,
             error::PrettyPrintError,
+            issue::PlainIssue,
             source_map::{GenerateSourceMap, Token},
-            version::{PartialUpdate, TotalUpdate, Update},
+            version::{PartialUpdate, TotalUpdate, Update, VersionState},
         },
         ecmascript_hmr_protocol::{ClientUpdateInstruction, ResourceIdentifier},
         trace_utils::{
@@ -424,6 +427,29 @@ struct NapiEntrypoints {
     pub pages_error_endpoint: External<ExternalEndpoint>,
 }
 
+#[turbo_tasks::value(serialization = "none")]
+struct EntrypointsWithIssues {
+    entrypoints: ReadRef<Entrypoints>,
+    issues: Arc<Vec<ReadRef<PlainIssue>>>,
+    diagnostics: Arc<Vec<ReadRef<PlainDiagnostic>>>,
+}
+
+#[turbo_tasks::function]
+async fn get_entrypoints_with_issues(
+    container: Vc<ProjectContainer>,
+) -> Result<Vc<EntrypointsWithIssues>> {
+    let entrypoints_operation = container.entrypoints();
+    let entrypoints = entrypoints_operation.strongly_consistent().await?;
+    let issues = get_issues(entrypoints_operation).await?;
+    let diagnostics = get_diagnostics(entrypoints_operation).await?;
+    Ok(EntrypointsWithIssues {
+        entrypoints,
+        issues,
+        diagnostics,
+    }
+    .cell())
+}
+
 #[napi(ts_return_type = "{ __napiType: \"RootTask\" }")]
 pub fn project_entrypoints_subscribe(
     #[napi(ts_arg_type = "{ __napiType: \"Project\" }")] project: External<ProjectInstance>,
@@ -436,13 +462,14 @@ pub fn project_entrypoints_subscribe(
         func,
         move || {
             async move {
-                let entrypoints_operation = container.entrypoints();
-                let entrypoints = entrypoints_operation.strongly_consistent().await?;
-
-                let issues = get_issues(entrypoints_operation).await?;
-                let diags = get_diagnostics(entrypoints_operation).await?;
-
-                Ok((entrypoints, issues, diags))
+                let EntrypointsWithIssues {
+                    entrypoints,
+                    issues,
+                    diagnostics,
+                } = &*get_entrypoints_with_issues(container)
+                    .strongly_consistent()
+                    .await?;
+                Ok((entrypoints.clone(), issues.clone(), diagnostics.clone()))
             }
             .instrument(tracing::info_span!("entrypoints subscription"))
         },
@@ -491,6 +518,31 @@ pub fn project_entrypoints_subscribe(
     )
 }
 
+#[turbo_tasks::value(serialization = "none")]
+struct HmrUpdateWithIssues {
+    update: ReadRef<Update>,
+    issues: Arc<Vec<ReadRef<PlainIssue>>>,
+    diagnostics: Arc<Vec<ReadRef<PlainDiagnostic>>>,
+}
+
+#[turbo_tasks::function]
+async fn hmr_update(
+    project: Vc<Project>,
+    identifier: String,
+    state: Vc<VersionState>,
+) -> Result<Vc<HmrUpdateWithIssues>> {
+    let update_operation = project.hmr_update(identifier, state);
+    let update = update_operation.strongly_consistent().await?;
+    let issues = get_issues(update_operation).await?;
+    let diagnostics = get_diagnostics(update_operation).await?;
+    Ok(HmrUpdateWithIssues {
+        update,
+        issues,
+        diagnostics,
+    }
+    .cell())
+}
+
 #[napi(ts_return_type = "{ __napiType: \"RootTask\" }")]
 pub fn project_hmr_events(
     #[napi(ts_arg_type = "{ __napiType: \"Project\" }")] project: External<ProjectInstance>,
@@ -510,14 +562,17 @@ pub fn project_hmr_events(
                 let identifier = outer_identifier.clone();
                 let session = session.clone();
                 async move {
-                    let state = project
-                        .project()
-                        .hmr_version_state(identifier.clone(), session);
-                    let update_operation = project.project().hmr_update(identifier, state);
-                    let update = update_operation.strongly_consistent().await?;
-                    let issues = get_issues(update_operation).await?;
-                    let diags = get_diagnostics(update_operation).await?;
-                    match &*update {
+                    let project = project.project().resolve().await?;
+                    let state = project.hmr_version_state(identifier.clone(), session);
+                    let update = hmr_update(project, identifier, state)
+                        .strongly_consistent()
+                        .await?;
+                    let HmrUpdateWithIssues {
+                        update,
+                        issues,
+                        diagnostics,
+                    } = &*update;
+                    match &**update {
                         Update::None => {}
                         Update::Total(TotalUpdate { to }) => {
                             state.set(to.clone()).await?;
@@ -526,7 +581,7 @@ pub fn project_hmr_events(
                             state.set(to.clone()).await?;
                         }
                     }
-                    Ok((update, issues, diags))
+                    Ok((update.clone(), issues.clone(), diagnostics.clone()))
                 }
                 .instrument(tracing::info_span!(
                     "HMR subscription",
@@ -574,6 +629,29 @@ struct HmrIdentifiers {
     pub identifiers: Vec<String>,
 }
 
+#[turbo_tasks::value(serialization = "none")]
+struct HmrIdentifiersWithIssues {
+    identifiers: ReadRef<Vec<String>>,
+    issues: Arc<Vec<ReadRef<PlainIssue>>>,
+    diagnostics: Arc<Vec<ReadRef<PlainDiagnostic>>>,
+}
+
+#[turbo_tasks::function]
+async fn get_hmr_identifiers_with_issues(
+    container: Vc<ProjectContainer>,
+) -> Result<Vc<HmrIdentifiersWithIssues>> {
+    let hmr_identifiers_operation = container.hmr_identifiers();
+    let hmr_identifiers = hmr_identifiers_operation.strongly_consistent().await?;
+    let issues = get_issues(hmr_identifiers_operation).await?;
+    let diagnostics = get_diagnostics(hmr_identifiers_operation).await?;
+    Ok(HmrIdentifiersWithIssues {
+        identifiers: hmr_identifiers,
+        issues,
+        diagnostics,
+    }
+    .cell())
+}
+
 #[napi(ts_return_type = "{ __napiType: \"RootTask\" }")]
 pub fn project_hmr_identifiers_subscribe(
     #[napi(ts_arg_type = "{ __napiType: \"Project\" }")] project: External<ProjectInstance>,
@@ -585,20 +663,22 @@ pub fn project_hmr_identifiers_subscribe(
         turbo_tasks.clone(),
         func,
         move || async move {
-            let hmr_identifiers_operation = container.hmr_identifiers();
-            let hmr_identifiers = hmr_identifiers_operation.strongly_consistent().await?;
-
-            let issues = get_issues(hmr_identifiers_operation).await?;
-            let diags = get_diagnostics(hmr_identifiers_operation).await?;
+            let HmrIdentifiersWithIssues {
+                identifiers,
+                issues,
+                diagnostics,
+            } = &*get_hmr_identifiers_with_issues(container)
+                .strongly_consistent()
+                .await?;
 
-            Ok((hmr_identifiers, issues, diags))
+            Ok((identifiers.clone(), issues.clone(), diagnostics.clone()))
         },
         move |ctx| {
-            let (hmr_identifiers, issues, diags) = ctx.value;
+            let (identifiers, issues, diagnostics) = ctx.value;
 
             Ok(vec![TurbopackResult {
                 result: HmrIdentifiers {
-                    identifiers: hmr_identifiers
+                    identifiers: identifiers
                         .iter()
                         .map(|ident| ident.to_string())
                         .collect::<Vec<_>>(),
@@ -607,7 +687,10 @@ pub fn project_hmr_identifiers_subscribe(
                     .iter()
                     .map(|issue| NapiIssue::from(&**issue))
                     .collect(),
-                diagnostics: diags.iter().map(|d| NapiDiagnostic::from(d)).collect(),
+                diagnostics: diagnostics
+                    .iter()
+                    .map(|d| NapiDiagnostic::from(d))
+                    .collect(),
             }])
         },
     )

packages/next-swc/crates/napi/src/next_api/utils.rs

@@ -77,22 +77,24 @@ pub fn root_task_dispose(
     Ok(())
 }
 
-pub async fn get_issues<T: Send>(source: Vc<T>) -> Result<Vec<ReadRef<PlainIssue>>> {
+pub async fn get_issues<T: Send>(source: Vc<T>) -> Result<Arc<Vec<ReadRef<PlainIssue>>>> {
     let issues = source.peek_issues_with_path().await?;
-    issues.get_plain_issues().await
+    Ok(Arc::new(issues.get_plain_issues().await?))
 }
 
 /// Collect [turbopack::core::diagnostics::Diagnostic] from given source,
 /// returns [turbopack::core::diagnostics::PlainDiagnostic]
-pub async fn get_diagnostics<T: Send>(source: Vc<T>) -> Result<Vec<ReadRef<PlainDiagnostic>>> {
+pub async fn get_diagnostics<T: Send>(source: Vc<T>) -> Result<Arc<Vec<ReadRef<PlainDiagnostic>>>> {
     let captured_diags = source.peek_diagnostics().await?;
 
-    captured_diags
-        .diagnostics
-        .iter()
-        .map(|d| d.into_plain())
-        .try_join()
-        .await
+    Ok(Arc::new(
+        captured_diags
+            .diagnostics
+            .iter()
+            .map(|d| d.into_plain())
+            .try_join()
+            .await?,
+    ))
 }
 
 #[napi(object)]

packages/next-swc/crates/next-api/src/lib.rs

@@ -3,7 +3,7 @@
 
 mod app;
 mod dynamic_imports;
-mod entrypoints;
+pub mod entrypoints;
 mod instrumentation;
 mod middleware;
 mod pages;

packages/next-swc/crates/next-api/src/server_actions.rs

@@ -56,7 +56,7 @@ pub(crate) async fn create_server_actions_manifest(
 ) -> Result<(Vc<Box<dyn EvaluatableAsset>>, Vc<Box<dyn OutputAsset>>)> {
     let actions = get_actions(rsc_entry, server_reference_modules, asset_context);
     let loader =
-        build_server_actions_loader(project_path, page_name, actions, asset_context).await?;
+        build_server_actions_loader(project_path, page_name.to_string(), actions, asset_context);
     let evaluable = Vc::try_resolve_sidecast::<Box<dyn EvaluatableAsset>>(loader)
         .await?
         .context("loader module must be evaluatable")?;
@@ -76,9 +76,10 @@ pub(crate) async fn create_server_actions_manifest(
 /// The actions are reexported under a hashed name (comprised of the exporting
 /// file's name and the action name). This hash matches the id sent to the
 /// client and present inside the paired manifest.
+#[turbo_tasks::function]
 async fn build_server_actions_loader(
     project_path: Vc<FileSystemPath>,
-    page_name: &str,
+    page_name: String,
     actions: Vc<AllActions>,
     asset_context: Vc<Box<dyn AssetContext>>,
 ) -> Result<Vc<Box<dyn EcmascriptChunkPlaceable>>> {