Reland "Refine the not-found rendering process for app router" (#52985)

Reland #52790 Reverts #52977 was failed due to failed job [vercel/next.js/actions/runs/5616458194/job/15220295829](https://github.com/vercel/next.js/actions/runs/5616458194/job/15220295829) Should be fine to resolve with #52979 now Fixes #52718 Fixes #52739 --------- Co-authored-by: Alex Kirszenberg <alex.kirszenberg@vercel.com> Co-authored-by: Tobias Koppers <tobias.koppers@googlemail.com>
vercel · Jul 21, 2023 · 1fefb4a · 1fefb4a
1 parent 732219e
commit 1fefb4a
Show file tree

Hide file tree

Showing 29 changed files with 448 additions and 280 deletions.
diff --git a/packages/next-swc/crates/next-core/js/src/dev/hot-reloader.tsx b/packages/next-swc/crates/next-core/js/src/dev/hot-reloader.tsx
@@ -5,22 +5,12 @@ import { useRouter, usePathname } from 'next/dist/client/components/navigation'
 import { useEffect } from 'react'
 import { subscribeToUpdate } from '@vercel/turbopack-ecmascript-runtime/dev/client/hmr-client'
 import { ReactDevOverlay } from './client'
-import { NotFoundBoundary } from 'next/dist/client/components/not-found-boundary'
 
 type HotReloadProps = React.PropsWithChildren<{
   assetPrefix?: string
-  notFound?: React.ReactNode
-  notFoundStyles?: React.ReactNode
-  asNotFound?: boolean
 }>
 
-export default function HotReload({
-  assetPrefix,
-  children,
-  notFound,
-  notFoundStyles,
-  asNotFound,
-}: HotReloadProps) {
+export default function HotReload({ children }: HotReloadProps) {
   const router = useRouter()
   const path = usePathname()!.slice(1)
 
@@ -41,14 +31,5 @@ export default function HotReload({
     return unsubscribe
   }, [router, path])
 
-  return (
-    <NotFoundBoundary
-      key={asNotFound + ''}
-      notFound={notFound}
-      notFoundStyles={notFoundStyles}
-      asNotFound={asNotFound}
-    >
-      <ReactDevOverlay globalOverlay={true}>{children}</ReactDevOverlay>
-    </NotFoundBoundary>
-  )
+  return <ReactDevOverlay globalOverlay={true}>{children}</ReactDevOverlay>
 }
diff --git a/packages/next-swc/crates/next-core/js/src/overlay/internal/ReactDevOverlay.tsx b/packages/next-swc/crates/next-core/js/src/overlay/internal/ReactDevOverlay.tsx
@@ -7,7 +7,6 @@ import { ErrorBoundary } from './ErrorBoundary'
 import { Base } from './styles/Base'
 import { ComponentStyles } from './styles/ComponentStyles'
 import { CssReset } from './styles/CssReset'
-import { notFound } from 'next/dist/client/components/not-found'
 
 type RefreshState =
   | {
@@ -210,10 +209,6 @@ export default function ReactDevOverlay({
 
   const isMounted = hasBuildError || hasRuntimeErrors
 
-  if (state.notFound) {
-    notFound()
-  }
-
   return (
     <React.Fragment>
       <ErrorBoundary

diff --git a/packages/next-swc/crates/next-core/src/app_source.rs b/packages/next-swc/crates/next-core/src/app_source.rs
@@ -601,7 +601,7 @@ pub async fn create_app_source(
         )))
         .collect();
 
-    if let Some(&Entrypoint::AppPage { loader_tree }) = entrypoints.get("/") {
+    if let Some(&Entrypoint::AppPage { loader_tree }) = entrypoints.get("/_not-found") {
         if loader_tree.await?.components.await?.not_found.is_some() {
             // Only add a source for the app 404 page if a top-level not-found page is
             // defined. Otherwise, the 404 page is handled by the pages logic.

diff --git a/packages/next-swc/crates/next-core/src/app_structure.rs b/packages/next-swc/crates/next-core/src/app_structure.rs
@@ -14,7 +14,7 @@ use turbopack_binding::{
     turbopack::core::issue::{Issue, IssueExt, IssueSeverity},
 };
 
-use crate::next_config::NextConfig;
+use crate::{next_config::NextConfig, next_import_map::get_next_package};
 
 /// A final route in the app directory.
 #[turbo_tasks::value]
@@ -675,9 +675,16 @@ async fn directory_tree_to_entrypoints_internal(
                 LoaderTree {
                     segment: directory_name.to_string(),
                     parallel_routes: indexmap! {
-                        // TODO(alexkirsz) Next.js has a __DEFAULT__ entry for
-                        // next/dist/client/components/parallel-route-default
-                        // here.
+                        "children".to_string() => LoaderTree {
+                            segment: "__DEFAULT__".to_string(),
+                            parallel_routes: IndexMap::new(),
+                            components: Components {
+                                default: Some(get_next_package(app_dir).join("dist/client/components/parallel-route-default.js".to_string())),
+                                ..Default::default()
+                            }
+                            .cell(),
+                        }
+                        .cell(),
                     },
                     components: components.without_leafs().cell(),
                 }

diff --git a/packages/next-swc/crates/next-dev-tests/tests/integration.rs b/packages/next-swc/crates/next-dev-tests/tests/integration.rs
@@ -3,6 +3,7 @@
 #![cfg(test)]
 
 use std::{
+    collections::{hash_map::Entry, HashMap},
     env,
     fmt::Write,
     future::{pending, Future},
@@ -34,6 +35,7 @@ use next_core::turbopack::{
 };
 use next_dev::{EntryRequest, NextDevServerBuilder};
 use owo_colors::OwoColorize;
+use parking_lot::Mutex;
 use regex::{Captures, Regex, Replacer};
 use serde::Deserialize;
 use tempdir::TempDir;
@@ -637,10 +639,12 @@ struct ChangeFileCommand {
     replace_with: String,
 }
 
-#[turbo_tasks::value(shared)]
+#[turbo_tasks::value(shared, serialization = "none", eq = "manual", cell = "new")]
 struct TestIssueReporter {
     #[turbo_tasks(trace_ignore, debug_ignore)]
     pub issue_tx: State<UnboundedSender<(ReadRef<PlainIssue>, ReadRef<ValueDebugString>)>>,
+    #[turbo_tasks(trace_ignore, debug_ignore)]
+    pub already_printed: Mutex<HashMap<String, ()>>,
 }
 
 #[turbo_tasks::value_impl]
@@ -653,6 +657,7 @@ impl TestIssueReporter {
     ) -> Vc<Self> {
         TestIssueReporter {
             issue_tx: State::new((*issue_tx).clone()),
+            already_printed: Default::default(),
         }
         .cell()
     }
@@ -678,7 +683,11 @@ impl IssueReporter for TestIssueReporter {
         for (issue, path) in captured_issues.iter_with_shortest_path() {
             let plain = NormalizedIssue(issue).cell().into_plain(path);
             issue_tx.send((plain.await?, plain.dbg().await?))?;
-            println!("{}", format_issue(&*plain.await?, None, &log_options));
+            let str = format_issue(&*plain.await?, None, &log_options);
+            if let Entry::Vacant(e) = self.already_printed.lock().entry(str) {
+                println!("{}", e.key());
+                e.insert(());
+            }
         }
         Ok(Vc::cell(false))
     }

diff --git a/...s/next-swc/crates/next-dev-tests/tests/integration/next/app/404-custom/input/app/test.tsx b/...s/next-swc/crates/next-dev-tests/tests/integration/next/app/404-custom/input/app/test.tsx
@@ -48,6 +48,7 @@ function runTests(harness: Harness, iframe: HTMLIFrameElement) {
     TIMEOUT
   )
 
+  // TODO: This test is flaky, so it needs a particularly long timeout.
   it(
     'renders a custom 404 page',
     async () => {
@@ -58,7 +59,7 @@ function runTests(harness: Harness, iframe: HTMLIFrameElement) {
         iframe.contentDocument!.querySelector('[data-test-notfound]')
       ).not.toBeNull()
     },
-    TIMEOUT
+    LONG_TIMEOUT
   )
 
   // TODO: This test is flaky, so it needs a particularly long timeout.

diff --git a/packages/next/src/client/components/app-router.tsx b/packages/next/src/client/components/app-router.tsx
@@ -54,7 +54,6 @@ import { isBot } from '../../shared/lib/router/utils/is-bot'
 import { addBasePath } from '../add-base-path'
 import { AppRouterAnnouncer } from './app-router-announcer'
 import { RedirectBoundary } from './redirect-boundary'
-import { NotFoundBoundary } from './not-found-boundary'
 import { findHeadInCache } from './router-reducer/reducers/find-head-in-cache'
 import { createInfinitePromise } from './infinite-promise'
 import { NEXT_RSC_UNION_QUERY } from './app-router-headers'
@@ -89,24 +88,13 @@ export function urlToUrlWithoutFlightMarker(url: string): URL {
   return urlWithoutFlightParameters
 }
 
-const HotReloader:
-  | typeof import('./react-dev-overlay/hot-reloader-client').default
-  | null =
-  process.env.NODE_ENV === 'production'
-    ? null
-    : (require('./react-dev-overlay/hot-reloader-client')
-        .default as typeof import('./react-dev-overlay/hot-reloader-client').default)
-
 type AppRouterProps = Omit<
   Omit<InitialRouterStateParameters, 'isServer' | 'location'>,
   'initialParallelRoutes'
 > & {
   buildId: string
   initialHead: ReactNode
   assetPrefix: string
-  // Top level boundaries props
-  notFound: React.ReactNode | undefined
-  asNotFound?: boolean
 }
 
 function isExternalURL(url: URL) {
@@ -224,8 +212,6 @@ function Router({
   initialCanonicalUrl,
   children,
   assetPrefix,
-  notFound,
-  asNotFound,
 }: AppRouterProps) {
   const initialState = useMemo(
     () =>
@@ -445,16 +431,26 @@ function Router({
     return findHeadInCache(cache, tree[1])
   }, [cache, tree])
 
-  const notFoundProps = { notFound, asNotFound }
-
-  const content = (
+  let content = (
     <RedirectBoundary>
       {head}
       {cache.subTreeData}
       <AppRouterAnnouncer tree={tree} />
     </RedirectBoundary>
   )
 
+  if (process.env.NODE_ENV !== 'production') {
+    if (typeof window !== 'undefined') {
+      const DevRootNotFoundBoundary: typeof import('./dev-root-not-found-boundary').DevRootNotFoundBoundary =
+        require('./dev-root-not-found-boundary').DevRootNotFoundBoundary
+      content = <DevRootNotFoundBoundary>{content}</DevRootNotFoundBoundary>
+    }
+    const HotReloader: typeof import('./react-dev-overlay/hot-reloader-client').default =
+      require('./react-dev-overlay/hot-reloader-client').default
+
+    content = <HotReloader assetPrefix={assetPrefix}>{content}</HotReloader>
+  }
+
   return (
     <>
       <HistoryUpdater
@@ -484,16 +480,7 @@ function Router({
                   url: canonicalUrl,
                 }}
               >
-                {HotReloader ? (
-                  // HotReloader implements a separate NotFoundBoundary to maintain the HMR ping interval
-                  <HotReloader assetPrefix={assetPrefix} {...notFoundProps}>
-                    {content}
-                  </HotReloader>
-                ) : (
-                  <NotFoundBoundary {...notFoundProps}>
-                    {content}
-                  </NotFoundBoundary>
-                )}
+                {content}
               </LayoutRouterContext.Provider>
             </AppRouterContext.Provider>
           </GlobalLayoutRouterContext.Provider>

diff --git a/packages/next/src/client/components/dev-root-not-found-boundary.tsx b/packages/next/src/client/components/dev-root-not-found-boundary.tsx
@@ -0,0 +1,25 @@
+'use client'
+
+import React from 'react'
+import { NotFoundBoundary } from './not-found-boundary'
+
+export function bailOnNotFound() {
+  throw new Error('notFound() is not allowed to use in root layout')
+}
+
+function NotAllowedRootNotFoundError() {
+  bailOnNotFound()
+  return null
+}
+
+export function DevRootNotFoundBoundary({
+  children,
+}: {
+  children: React.ReactNode
+}) {
+  return (
+    <NotFoundBoundary notFound={<NotAllowedRootNotFoundError />}>
+      {children}
+    </NotFoundBoundary>
+  )
+}
diff --git a/packages/next/src/client/components/layout-router.tsx b/packages/next/src/client/components/layout-router.tsx
@@ -491,7 +491,6 @@ export default function OuterLayoutRouter({
   template,
   notFound,
   notFoundStyles,
-  asNotFound,
   styles,
 }: {
   parallelRouterKey: string
@@ -506,7 +505,6 @@ export default function OuterLayoutRouter({
   hasLoading: boolean
   notFound: React.ReactNode | undefined
   notFoundStyles: React.ReactNode | undefined
-  asNotFound?: boolean
   styles?: React.ReactNode
 }) {
   const context = useContext(LayoutRouterContext)
@@ -574,7 +572,6 @@ export default function OuterLayoutRouter({
                     <NotFoundBoundary
                       notFound={notFound}
                       notFoundStyles={notFoundStyles}
-                      asNotFound={asNotFound}
                     >
                       <RedirectBoundary>
                         <InnerLayoutRouter

diff --git a/packages/next/src/client/components/not-found-boundary.tsx b/packages/next/src/client/components/not-found-boundary.tsx
@@ -64,6 +64,9 @@ class NotFoundErrorBoundary extends React.Component<
       return (
         <>
           <meta name="robots" content="noindex" />
+          {process.env.NODE_ENV === 'development' && (
+            <meta name="next-error" content="not-found" />
+          )}
           {this.props.notFoundStyles}
           {this.props.notFound}
         </>