diff --git a/crates/napi/src/next_api/endpoint.rs b/crates/napi/src/next_api/endpoint.rs index 74712fa585893..9aa75aa661d60 100644 --- a/crates/napi/src/next_api/endpoint.rs +++ b/crates/napi/src/next_api/endpoint.rs @@ -3,10 +3,11 @@ use std::{ops::Deref, sync::Arc}; use anyhow::Result; use napi::{bindgen_prelude::External, JsFunction}; use next_api::{ + operation::OptionEndpoint, paths::ServerPath, route::{ - endpoint_server_changed_operation, endpoint_write_to_disk_operation, Endpoint, - EndpointOutputPaths, + endpoint_client_changed_operation, endpoint_server_changed_operation, + endpoint_write_to_disk_operation, EndpointOutputPaths, }, }; use tracing::Instrument; @@ -71,7 +72,7 @@ impl From> for NapiWrittenEndpoint { server_paths: server_paths.into_iter().map(From::from).collect(), ..Default::default() }, - None => Self { + Some(EndpointOutputPaths::NotFound) | None => Self { r#type: "none".to_string(), ..Default::default() }, @@ -85,10 +86,10 @@ impl From> 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>); +pub struct ExternalEndpoint(pub VcArc); impl Deref for ExternalEndpoint { - type Target = VcArc>; + type Target = VcArc; fn deref(&self) -> &Self::Target { &self.0 @@ -105,7 +106,7 @@ struct WrittenEndpointWithIssues { #[turbo_tasks::function(operation)] async fn get_written_endpoint_with_issues_operation( - endpoint_op: OperationVc>, + endpoint_op: OperationVc, ) -> Result> { let write_to_disk_op = endpoint_write_to_disk_operation(endpoint_op); let (written, issues, diagnostics, effects) = @@ -214,7 +215,7 @@ impl Eq for EndpointIssuesAndDiags {} #[turbo_tasks::function(operation)] async fn subscribe_issues_and_diags_operation( - endpoint_op: OperationVc>, + endpoint_op: OperationVc, should_include_issues: bool, ) -> Result> { let changed_op = endpoint_server_changed_operation(endpoint_op); @@ -241,13 +242,6 @@ async fn subscribe_issues_and_diags_operation( } } -#[turbo_tasks::function(operation)] -fn endpoint_client_changed_operation( - endpoint_op: OperationVc>, -) -> Vc { - endpoint_op.connect().client_changed() -} - #[napi(ts_return_type = "{ __napiType: \"RootTask\" }")] pub fn endpoint_client_changed_subscribe( #[napi(ts_arg_type = "{ __napiType: \"Endpoint\" }")] endpoint: External, diff --git a/crates/napi/src/next_api/project.rs b/crates/napi/src/next_api/project.rs index 2dc4f553f953f..155bbb70b5831 100644 --- a/crates/napi/src/next_api/project.rs +++ b/crates/napi/src/next_api/project.rs @@ -9,7 +9,8 @@ use napi::{ use next_api::{ entrypoints::Entrypoints, operation::{ - EntrypointsOperation, InstrumentationOperation, MiddlewareOperation, RouteOperation, + EntrypointsOperation, InstrumentationOperation, MiddlewareOperation, OptionEndpoint, + RouteOperation, }, project::{ DefineEnv, DraftModeOptions, PartialProjectOptions, Project, ProjectContainer, @@ -580,7 +581,7 @@ pub struct NapiRoute { impl NapiRoute { fn from_route(pathname: String, value: RouteOperation, turbo_tasks: &NextTurboTasks) -> Self { - let convert_endpoint = |endpoint: OperationVc>| { + let convert_endpoint = |endpoint: OperationVc| { Some(External::new(ExternalEndpoint(VcArc::new( turbo_tasks.clone(), endpoint, diff --git a/crates/next-api/src/operation.rs b/crates/next-api/src/operation.rs index db04f0928a696..ad9c28667d015 100644 --- a/crates/next-api/src/operation.rs +++ b/crates/next-api/src/operation.rs @@ -3,7 +3,7 @@ use serde::{Deserialize, Serialize}; use turbo_rcstr::RcStr; use turbo_tasks::{ debug::ValueDebugFormat, get_effects, trace::TraceRawVcs, CollectiblesSource, FxIndexMap, - NonLocalValue, OperationVc, ResolvedVc, Vc, + NonLocalValue, OperationValue, OperationVc, ResolvedVc, TaskInput, Vc, }; use turbopack_core::{diagnostics::Diagnostic, issue::IssueDescriptionExt}; @@ -25,9 +25,9 @@ pub struct EntrypointsOperation { pub routes: FxIndexMap, pub middleware: Option, pub instrumentation: Option, - pub pages_document_endpoint: OperationVc>, - pub pages_app_endpoint: OperationVc>, - pub pages_error_endpoint: OperationVc>, + pub pages_document_endpoint: OperationVc, + pub pages_app_endpoint: OperationVc, + pub pages_error_endpoint: OperationVc, } /// Removes diagnostics, issues, and effects from the top-level `entrypoints` operation so that @@ -53,96 +53,181 @@ impl EntrypointsOperation { routes: e .routes .iter() - .map(|(k, v)| (k.clone(), wrap_route(v, entrypoints))) + .map(|(k, v)| (k.clone(), pick_route(entrypoints, k.clone(), v))) .collect(), - middleware: e.middleware.as_ref().map(|m| MiddlewareOperation { - endpoint: wrap(m.endpoint, entrypoints), + middleware: e.middleware.as_ref().map(|_| MiddlewareOperation { + endpoint: pick_endpoint(entrypoints, EndpointSelector::Middleware), }), instrumentation: e .instrumentation .as_ref() - .map(|i| InstrumentationOperation { - node_js: wrap(i.node_js, entrypoints), - edge: wrap(i.edge, entrypoints), + .map(|_| InstrumentationOperation { + node_js: pick_endpoint(entrypoints, EndpointSelector::InstrumentationNodeJs), + edge: pick_endpoint(entrypoints, EndpointSelector::InstrumentationEdge), }), - pages_document_endpoint: wrap(e.pages_document_endpoint, entrypoints), - pages_app_endpoint: wrap(e.pages_app_endpoint, entrypoints), - pages_error_endpoint: wrap(e.pages_error_endpoint, entrypoints), + pages_document_endpoint: pick_endpoint(entrypoints, EndpointSelector::PagesDocument), + pages_app_endpoint: pick_endpoint(entrypoints, EndpointSelector::PagesApp), + pages_error_endpoint: pick_endpoint(entrypoints, EndpointSelector::PagesError), } .cell()) } } -fn wrap_route(route: &Route, entrypoints: OperationVc) -> RouteOperation { +fn pick_route(entrypoints: OperationVc, key: RcStr, route: &Route) -> RouteOperation { match route { - Route::Page { - html_endpoint, - data_endpoint, - } => RouteOperation::Page { - html_endpoint: wrap(*html_endpoint, entrypoints), - data_endpoint: wrap(*data_endpoint, entrypoints), + Route::Page { .. } => RouteOperation::Page { + html_endpoint: pick_endpoint(entrypoints, EndpointSelector::RoutePageHtml(key.clone())), + data_endpoint: pick_endpoint(entrypoints, EndpointSelector::RoutePageData(key)), }, - Route::PageApi { endpoint } => RouteOperation::PageApi { - endpoint: wrap(*endpoint, entrypoints), + Route::PageApi { .. } => RouteOperation::PageApi { + endpoint: pick_endpoint(entrypoints, EndpointSelector::RoutePageApi(key)), }, Route::AppPage(pages) => RouteOperation::AppPage( pages .iter() - .map(|p| AppPageRouteOperation { + .enumerate() + .map(|(i, p)| AppPageRouteOperation { original_name: p.original_name.clone(), - html_endpoint: wrap(p.html_endpoint, entrypoints), - rsc_endpoint: wrap(p.rsc_endpoint, entrypoints), + html_endpoint: pick_endpoint( + entrypoints, + EndpointSelector::RouteAppPageHtml(key.clone(), i), + ), + rsc_endpoint: pick_endpoint( + entrypoints, + EndpointSelector::RouteAppPageRsc(key.clone(), i), + ), }) .collect(), ), - Route::AppRoute { - original_name, - endpoint, - } => RouteOperation::AppRoute { + Route::AppRoute { original_name, .. } => RouteOperation::AppRoute { original_name: original_name.clone(), - endpoint: wrap(*endpoint, entrypoints), + endpoint: pick_endpoint(entrypoints, EndpointSelector::RouteAppRoute(key)), }, Route::Conflict => RouteOperation::Conflict, } } -/// Given a resolved `Endpoint` and the `Entrypoints` operation that it comes from, connect the -/// operation and return a `OperationVc` of the `Entrypoint`. This `Endpoint` operation will keep -/// the entire `Entrypoints` operation alive. +#[derive( + Debug, + Clone, + TaskInput, + Serialize, + Deserialize, + TraceRawVcs, + PartialEq, + Eq, + Hash, + ValueDebugFormat, + NonLocalValue, + OperationValue, +)] +enum EndpointSelector { + RoutePageHtml(RcStr), + RoutePageData(RcStr), + RoutePageApi(RcStr), + RouteAppPageHtml(RcStr, usize), + RouteAppPageRsc(RcStr, usize), + RouteAppRoute(RcStr), + InstrumentationNodeJs, + InstrumentationEdge, + Middleware, + PagesDocument, + PagesApp, + PagesError, +} + +#[turbo_tasks::value(transparent)] +pub struct OptionEndpoint(Option>>); + +/// Given a selector and the `Entrypoints` operation that it comes from, connect the operation and +/// return an `OperationVc` containing the selected value. The returned operation will keep the +/// entire `Entrypoints` operation alive. #[turbo_tasks::function(operation)] -fn wrap( - endpoint: ResolvedVc>, +async fn pick_endpoint( op: OperationVc, -) -> Vc> { - let _ = op.connect(); - *endpoint + selector: EndpointSelector, +) -> Result> { + let endpoints = op.connect().strongly_consistent().await?; + let endpoint = match selector { + EndpointSelector::InstrumentationNodeJs => { + endpoints.instrumentation.as_ref().map(|i| i.node_js) + } + EndpointSelector::InstrumentationEdge => endpoints.instrumentation.as_ref().map(|i| i.edge), + EndpointSelector::Middleware => endpoints.middleware.as_ref().map(|m| m.endpoint), + EndpointSelector::PagesDocument => Some(endpoints.pages_document_endpoint), + EndpointSelector::PagesApp => Some(endpoints.pages_app_endpoint), + EndpointSelector::PagesError => Some(endpoints.pages_error_endpoint), + EndpointSelector::RoutePageHtml(name) => { + if let Some(Route::Page { html_endpoint, .. }) = endpoints.routes.get(&name) { + Some(*html_endpoint) + } else { + None + } + } + EndpointSelector::RoutePageData(name) => { + if let Some(Route::Page { data_endpoint, .. }) = endpoints.routes.get(&name) { + Some(*data_endpoint) + } else { + None + } + } + EndpointSelector::RoutePageApi(name) => { + if let Some(Route::PageApi { endpoint }) = endpoints.routes.get(&name) { + Some(*endpoint) + } else { + None + } + } + EndpointSelector::RouteAppPageHtml(name, i) => { + if let Some(Route::AppPage(pages)) = endpoints.routes.get(&name) { + pages.get(i).as_ref().map(|p| p.html_endpoint) + } else { + None + } + } + EndpointSelector::RouteAppPageRsc(name, i) => { + if let Some(Route::AppPage(pages)) = endpoints.routes.get(&name) { + pages.get(i).as_ref().map(|p| p.rsc_endpoint) + } else { + None + } + } + EndpointSelector::RouteAppRoute(name) => { + if let Some(Route::AppRoute { endpoint, .. }) = endpoints.routes.get(&name) { + Some(*endpoint) + } else { + None + } + } + }; + Ok(Vc::cell(endpoint)) } #[derive(Serialize, Deserialize, TraceRawVcs, PartialEq, Eq, ValueDebugFormat, NonLocalValue)] pub struct InstrumentationOperation { - pub node_js: OperationVc>, - pub edge: OperationVc>, + pub node_js: OperationVc, + pub edge: OperationVc, } #[derive(Serialize, Deserialize, TraceRawVcs, PartialEq, Eq, ValueDebugFormat, NonLocalValue)] pub struct MiddlewareOperation { - pub endpoint: OperationVc>, + pub endpoint: OperationVc, } #[turbo_tasks::value(shared)] #[derive(Clone, Debug)] pub enum RouteOperation { Page { - html_endpoint: OperationVc>, - data_endpoint: OperationVc>, + html_endpoint: OperationVc, + data_endpoint: OperationVc, }, PageApi { - endpoint: OperationVc>, + endpoint: OperationVc, }, AppPage(Vec), AppRoute { original_name: RcStr, - endpoint: OperationVc>, + endpoint: OperationVc, }, Conflict, } @@ -160,6 +245,6 @@ pub enum RouteOperation { )] pub struct AppPageRouteOperation { pub original_name: RcStr, - pub html_endpoint: OperationVc>, - pub rsc_endpoint: OperationVc>, + pub html_endpoint: OperationVc, + pub rsc_endpoint: OperationVc, } diff --git a/crates/next-api/src/route.rs b/crates/next-api/src/route.rs index 987e36a511ad7..c6baa51efd465 100644 --- a/crates/next-api/src/route.rs +++ b/crates/next-api/src/route.rs @@ -10,7 +10,7 @@ use turbopack_core::{ output::OutputAssets, }; -use crate::{paths::ServerPath, project::Project}; +use crate::{operation::OptionEndpoint, paths::ServerPath, project::Project}; #[derive( TraceRawVcs, @@ -97,17 +97,36 @@ async fn endpoint_output_assets_operation( } #[turbo_tasks::function(operation)] -pub fn endpoint_write_to_disk_operation( - endpoint: OperationVc>, -) -> Vc { - endpoint_write_to_disk(endpoint.connect()) +pub async fn endpoint_write_to_disk_operation( + endpoint: OperationVc, +) -> Result> { + Ok(if let Some(endpoint) = *endpoint.connect().await? { + endpoint_write_to_disk(*endpoint) + } else { + EndpointOutputPaths::NotFound.cell() + }) +} + +#[turbo_tasks::function(operation)] +pub async fn endpoint_server_changed_operation( + endpoint: OperationVc, +) -> Result> { + Ok(if let Some(endpoint) = *endpoint.connect().await? { + endpoint.server_changed() + } else { + Completion::new() + }) } #[turbo_tasks::function(operation)] -pub fn endpoint_server_changed_operation( - endpoint: OperationVc>, -) -> Vc { - endpoint.connect().server_changed() +pub async fn endpoint_client_changed_operation( + endpoint: OperationVc, +) -> Result> { + Ok(if let Some(endpoint) = *endpoint.connect().await? { + endpoint.client_changed() + } else { + Completion::new() + }) } #[turbo_tasks::value(shared)] @@ -131,6 +150,7 @@ pub enum EndpointOutputPaths { server_paths: Vec, client_paths: Vec, }, + NotFound, } /// The routes as map from pathname to route. (pathname includes the leading diff --git a/test/development/app-hmr/hmr.test.ts b/test/development/app-hmr/hmr.test.ts index 63a4a99a2361f..0da35d0bb65d8 100644 --- a/test/development/app-hmr/hmr.test.ts +++ b/test/development/app-hmr/hmr.test.ts @@ -50,6 +50,8 @@ describe(`app-dir-hmr`, () => { // The new page should be rendered const newHTML = await next.render('/folder-renamed') expect(newHTML).toContain('Hello') + + expect(next.cliOutput).not.toContain('FATAL') } finally { // Rename it back await next.renameFolder('app/folder-renamed', 'app/folder') @@ -89,7 +91,6 @@ describe(`app-dir-hmr`, () => { // details are unimportant. expect(fastRefreshLogs).toEqual( expect.arrayContaining([ - { source: 'log', message: '[Fast Refresh] rebuilding' }, { source: 'log', message: expect.stringContaining('[Fast Refresh] done in '), @@ -102,6 +103,8 @@ describe(`app-dir-hmr`, () => { await retry(async () => { expect(await browser.elementByCss('p').text()).toBe('mac') }) + + expect(next.cliOutput).not.toContain('FATAL') }) it.each(['node', 'node-module-var', 'edge', 'edge-module-var'])( @@ -118,7 +121,7 @@ describe(`app-dir-hmr`, () => { expect(logs).toEqual( expect.arrayContaining([ expect.objectContaining({ - message: '[Fast Refresh] rebuilding', + message: expect.stringContaining('[Fast Refresh] done'), source: 'log', }), ]) @@ -143,6 +146,8 @@ describe(`app-dir-hmr`, () => { await retry(async () => { expect(await browser.elementByCss('p').text()).toBe('mac') }) + + expect(next.cliOutput).not.toContain('FATAL') } ) @@ -168,10 +173,6 @@ describe(`app-dir-hmr`, () => { // TODO: Should assert on all logs but these are cluttered with logs from our test utils (e.g. playwright tracing or webdriver) expect(logs).toEqual( expect.arrayContaining([ - { - message: '[Fast Refresh] rebuilding', - source: 'log', - }, { message: expect.stringContaining('[Fast Refresh] done in'), source: 'log', @@ -187,6 +188,8 @@ describe(`app-dir-hmr`, () => { ) // No MPA navigation triggered expect(await browser.eval('window.__TEST_NO_RELOAD')).toEqual(true) + + expect(next.cliOutput).not.toContain('FATAL') }) }) })