Revert "Update metadata ready tracking" (vercel#68200)

Reverts vercel#67929 This is causing deployment test failures. [x-ref](https://github.com/vercel/next.js/actions/runs/10103034192/job/27940614533#step:27:178) Validated that it passes locally. For reviewers, you can validate by comparing these 2 URLs: <details> <summary>Not Working</summary> https://vtest314-e2e-tests-6j3mwq9jg-ztanner.vercel.app/metadata-error-with-boundary </details> <details> <summary>Working</summary> https://vtest314-e2e-tests-ihcxlytpd-ztanner.vercel.app/metadata-error-with-boundary </details>
panteliselef · Jul 26, 2024 · 6fe66a0 · 6fe66a0
1 parent f43c53a
commit 6fe66a0
Show file tree

Hide file tree

Showing 4 changed files with 95 additions and 157 deletions.
diff --git a/packages/next/src/lib/metadata/metadata.tsx b/packages/next/src/lib/metadata/metadata.tsx
@@ -28,6 +28,10 @@ import type {
   ResolvedMetadata,
   ResolvedViewport,
 } from './types/metadata-interface'
+import {
+  createDefaultMetadata,
+  createDefaultViewport,
+} from './default-metadata'
 import { isNotFoundError } from '../../client/components/not-found'
 import type { MetadataContext } from './types/resolvers'
 
@@ -66,142 +70,96 @@ export function createMetadataComponents({
   createDynamicallyTrackedSearchParams: (
     searchParams: ParsedUrlQuery
   ) => ParsedUrlQuery
-}): [React.ComponentType, () => Promise<void>] {
-  let currentMetadataReady:
-    | null
-    | (Promise<void> & {
-        status?: string
-        value?: unknown
-      }) = null
+}): [React.ComponentType, React.ComponentType] {
+  let resolve: (value: Error | undefined) => void | undefined
+  // Only use promise.resolve here to avoid unhandled rejections
+  const metadataErrorResolving = new Promise<Error | undefined>((res) => {
+    resolve = res
+  })
 
   async function MetadataTree() {
-    const pendingMetadata = getResolvedMetadata(
-      tree,
-      query,
-      getDynamicParamFromSegment,
-      metadataContext,
-      createDynamicallyTrackedSearchParams,
-      errorType
-    )
+    const defaultMetadata = createDefaultMetadata()
+    const defaultViewport = createDefaultViewport()
+    let metadata: ResolvedMetadata | undefined = defaultMetadata
+    let viewport: ResolvedViewport | undefined = defaultViewport
+    let error: any
+    const errorMetadataItem: [null, null, null] = [null, null, null]
+    const errorConvention = errorType === 'redirect' ? undefined : errorType
+    const searchParams = createDynamicallyTrackedSearchParams(query)
 
-    // We construct this instrumented promise to allow React.use to synchronously unwrap
-    // it if it has already settled.
-    const metadataReady: Promise<void> & { status: string; value: unknown } =
-      pendingMetadata.then(
-        ([error]) => {
-          if (error) {
-            metadataReady.status = 'rejected'
-            metadataReady.value = error
-            throw error
-          }
-          metadataReady.status = 'fulfilled'
-          metadataReady.value = undefined
-        },
-        (error) => {
-          metadataReady.status = 'rejected'
-          metadataReady.value = error
-          throw error
-        }
-      ) as Promise<void> & { status: string; value: unknown }
-    metadataReady.status = 'pending'
-    currentMetadataReady = metadataReady
+    const [resolvedError, resolvedMetadata, resolvedViewport] =
+      await resolveMetadata({
+        tree,
+        parentParams: {},
+        metadataItems: [],
+        errorMetadataItem,
+        searchParams,
+        getDynamicParamFromSegment,
+        errorConvention,
+        metadataContext,
+      })
+    if (!resolvedError) {
+      viewport = resolvedViewport
+      metadata = resolvedMetadata
+      resolve(undefined)
+    } else {
+      error = resolvedError
+      // If a not-found error is triggered during metadata resolution, we want to capture the metadata
+      // for the not-found route instead of whatever triggered the error. For all error types, we resolve an
+      // error, which will cause the outlet to throw it so it'll be handled by an error boundary
+      // (either an actual error, or an internal error that renders UI such as the NotFoundBoundary).
+      if (!errorType && isNotFoundError(resolvedError)) {
+        const [notFoundMetadataError, notFoundMetadata, notFoundViewport] =
+          await resolveMetadata({
+            tree,
+            parentParams: {},
+            metadataItems: [],
+            errorMetadataItem,
+            searchParams,
+            getDynamicParamFromSegment,
+            errorConvention: 'not-found',
+            metadataContext,
+          })
+        viewport = notFoundViewport
+        metadata = notFoundMetadata
+        error = notFoundMetadataError || error
+      }
+      resolve(error)
+    }
 
-    // We ignore any error from metadata here because it needs to be thrown from within the Page
-    // not where the metadata itself is actually rendered
-    const [, elements] = await pendingMetadata
+    const elements = MetaFilter([
+      ViewportMeta({ viewport: viewport }),
+      BasicMeta({ metadata }),
+      AlternatesMetadata({ alternates: metadata.alternates }),
+      ItunesMeta({ itunes: metadata.itunes }),
+      FacebookMeta({ facebook: metadata.facebook }),
+      FormatDetectionMeta({ formatDetection: metadata.formatDetection }),
+      VerificationMeta({ verification: metadata.verification }),
+      AppleWebAppMeta({ appleWebApp: metadata.appleWebApp }),
+      OpenGraphMetadata({ openGraph: metadata.openGraph }),
+      TwitterMetadata({ twitter: metadata.twitter }),
+      AppLinksMeta({ appLinks: metadata.appLinks }),
+      IconsMetadata({ icons: metadata.icons }),
+    ])
+
+    if (appUsingSizeAdjustment) elements.push(<meta name="next-size-adjust" />)
 
     return (
       <>
         {elements.map((el, index) => {
           return React.cloneElement(el as React.ReactElement, { key: index })
         })}
-        {appUsingSizeAdjustment ? <meta name="next-size-adjust" /> : null}
       </>
     )
   }
 
-  function getMetadataReady() {
-    return Promise.resolve().then(() => {
-      if (currentMetadataReady) {
-        return currentMetadataReady
-      }
-      throw new Error(
-        'getMetadataReady was called before MetadataTree rendered'
-      )
-    })
-  }
-
-  return [MetadataTree, getMetadataReady]
-}
-
-async function getResolvedMetadata(
-  tree: LoaderTree,
-  query: ParsedUrlQuery,
-  getDynamicParamFromSegment: GetDynamicParamFromSegment,
-  metadataContext: MetadataContext,
-  createDynamicallyTrackedSearchParams: (
-    searchParams: ParsedUrlQuery
-  ) => ParsedUrlQuery,
-  errorType?: 'not-found' | 'redirect'
-): Promise<[any, Array<React.ReactNode>]> {
-  const errorMetadataItem: [null, null, null] = [null, null, null]
-  const errorConvention = errorType === 'redirect' ? undefined : errorType
-  const searchParams = createDynamicallyTrackedSearchParams(query)
-
-  const [error, metadata, viewport] = await resolveMetadata({
-    tree,
-    parentParams: {},
-    metadataItems: [],
-    errorMetadataItem,
-    searchParams,
-    getDynamicParamFromSegment,
-    errorConvention,
-    metadataContext,
-  })
-  if (!error) {
-    return [null, createMetadataElements(metadata, viewport)]
-  } else {
-    // If a not-found error is triggered during metadata resolution, we want to capture the metadata
-    // for the not-found route instead of whatever triggered the error. For all error types, we resolve an
-    // error, which will cause the outlet to throw it so it'll be handled by an error boundary
-    // (either an actual error, or an internal error that renders UI such as the NotFoundBoundary).
-    if (!errorType && isNotFoundError(error)) {
-      const [notFoundMetadataError, notFoundMetadata, notFoundViewport] =
-        await resolveMetadata({
-          tree,
-          parentParams: {},
-          metadataItems: [],
-          errorMetadataItem,
-          searchParams,
-          getDynamicParamFromSegment,
-          errorConvention: 'not-found',
-          metadataContext,
-        })
-      return [
-        notFoundMetadataError || error,
-        createMetadataElements(notFoundMetadata, notFoundViewport),
-      ]
+  async function MetadataOutlet() {
+    const error = await metadataErrorResolving
+    if (error) {
+      throw error
     }
-    return [error, []]
+    return null
   }
-}
 
-function createMetadataElements(
-  metadata: ResolvedMetadata,
-  viewport: ResolvedViewport
-) {
-  return MetaFilter([
-    ViewportMeta({ viewport: viewport }),
-    BasicMeta({ metadata }),
-    AlternatesMetadata({ alternates: metadata.alternates }),
-    ItunesMeta({ itunes: metadata.itunes }),
-    FacebookMeta({ facebook: metadata.facebook }),
-    FormatDetectionMeta({ formatDetection: metadata.formatDetection }),
-    VerificationMeta({ verification: metadata.verification }),
-    AppleWebAppMeta({ appleWebApp: metadata.appleWebApp }),
-    OpenGraphMetadata({ openGraph: metadata.openGraph }),
-    TwitterMetadata({ twitter: metadata.twitter }),
-    AppLinksMeta({ appLinks: metadata.appLinks }),
-    IconsMetadata({ icons: metadata.icons }),
-  ])
+  return [MetadataTree, MetadataOutlet]
 }
diff --git a/packages/next/src/server/app-render/app-render.tsx b/packages/next/src/server/app-render/app-render.tsx
@@ -350,7 +350,7 @@ async function generateDynamicRSCPayload(
   if (!options?.skipFlight) {
     const preloadCallbacks: PreloadCallbacks = []
 
-    const [MetadataTree, getMetadataReady] = createMetadataComponents({
+    const [MetadataTree, MetadataOutlet] = createMetadataComponents({
       tree: loaderTree,
       query,
       metadataContext: createMetadataContext(url.pathname, ctx.renderOpts),
@@ -379,7 +379,7 @@ async function generateDynamicRSCPayload(
         injectedFontPreloadTags: new Set(),
         rootLayoutIncluded: false,
         asNotFound: ctx.isNotFoundPath || options?.asNotFound,
-        getMetadataReady,
+        metadataOutlet: <MetadataOutlet />,
         preloadCallbacks,
       })
     ).map((path) => path.slice(1)) // remove the '' (root) segment
@@ -486,7 +486,7 @@ async function getRSCPayload(
     query
   )
 
-  const [MetadataTree, getMetadataReady] = createMetadataComponents({
+  const [MetadataTree, MetadataOutlet] = createMetadataComponents({
     tree,
     errorType: asNotFound ? 'not-found' : undefined,
     query,
@@ -509,7 +509,7 @@ async function getRSCPayload(
     injectedFontPreloadTags,
     rootLayoutIncluded: false,
     asNotFound: asNotFound,
-    getMetadataReady,
+    metadataOutlet: <MetadataOutlet />,
     missingSlots,
     preloadCallbacks,
   })

diff --git a/packages/next/src/server/app-render/create-component-tree.tsx b/packages/next/src/server/app-render/create-component-tree.tsx
@@ -34,7 +34,7 @@ export function createComponentTree(props: {
   injectedJS: Set<string>
   injectedFontPreloadTags: Set<string>
   asNotFound?: boolean
-  getMetadataReady: () => Promise<void>
+  metadataOutlet?: React.ReactNode
   ctx: AppRenderContext
   missingSlots?: Set<string>
   preloadCallbacks: PreloadCallbacks
@@ -64,7 +64,7 @@ async function createComponentTreeInternal({
   injectedJS,
   injectedFontPreloadTags,
   asNotFound,
-  getMetadataReady,
+  metadataOutlet,
   ctx,
   missingSlots,
   preloadCallbacks,
@@ -78,7 +78,7 @@ async function createComponentTreeInternal({
   injectedJS: Set<string>
   injectedFontPreloadTags: Set<string>
   asNotFound?: boolean
-  getMetadataReady: () => Promise<void>
+  metadataOutlet?: React.ReactNode
   ctx: AppRenderContext
   missingSlots?: Set<string>
   preloadCallbacks: PreloadCallbacks
@@ -436,12 +436,10 @@ async function createComponentTreeInternal({
             injectedJS: injectedJSWithCurrentLayout,
             injectedFontPreloadTags: injectedFontPreloadTagsWithCurrentLayout,
             asNotFound,
-            // The metadataReady is used to trigger any errors that were caught during metadata resolution.
-            // We only want to throw once per segment, as otherwise the error will be triggered
+            // The metadataOutlet is responsible for throwing any errors that were caught during metadata resolution.
+            // We only want to render an outlet once per segment, as otherwise the error will be triggered
             // multiple times causing an uncaught error.
-            getMetadataReady: isChildrenRouteKey
-              ? getMetadataReady
-              : () => Promise.resolve(),
+            metadataOutlet: isChildrenRouteKey ? metadataOutlet : undefined,
             ctx,
             missingSlots,
             preloadCallbacks,
@@ -589,7 +587,7 @@ async function createComponentTreeInternal({
       props.searchParams = createUntrackedSearchParams(query)
       segmentElement = (
         <>
-          <MetadataOutlet getReady={getMetadataReady} />
+          {metadataOutlet}
           <ClientPageRoot props={props} Component={Component} />
           {layerAssets}
         </>
@@ -600,7 +598,7 @@ async function createComponentTreeInternal({
       props.searchParams = createDynamicallyTrackedSearchParams(query)
       segmentElement = (
         <>
-          <MetadataOutlet getReady={getMetadataReady} />
+          {metadataOutlet}
           <Component {...props} />
           {layerAssets}
         </>
@@ -634,21 +632,3 @@ async function createComponentTreeInternal({
     loadingData,
   ]
 }
-
-async function MetadataOutlet({
-  getReady,
-}: {
-  getReady: () => Promise<void> & { status?: string; value?: unknown }
-}) {
-  const ready = getReady()
-  // We actually expect this to be an instrumented promise and once this file is properly
-  // moved to the RSC module graph we can switch to using React.use for this synchronous unwrapping.
-  // The synchronous unwrapping will become important with dynamic IO since we want to resolve metadata
-  // before anything dynamic can be triggered
-  if (ready.status === 'rejected') {
-    throw ready.value
-  } else if (ready.status !== 'fulfilled') {
-    await ready
-  }
-  return null
-}
diff --git a/packages/next/src/server/app-render/walk-tree-with-flight-router-state.tsx b/packages/next/src/server/app-render/walk-tree-with-flight-router-state.tsx
@@ -38,7 +38,7 @@ export async function walkTreeWithFlightRouterState({
   injectedFontPreloadTags,
   rootLayoutIncluded,
   asNotFound,
-  getMetadataReady,
+  metadataOutlet,
   ctx,
   preloadCallbacks,
 }: {
@@ -54,7 +54,7 @@ export async function walkTreeWithFlightRouterState({
   injectedFontPreloadTags: Set<string>
   rootLayoutIncluded: boolean
   asNotFound?: boolean
-  getMetadataReady: () => Promise<void>
+  metadataOutlet: React.ReactNode
   ctx: AppRenderContext
   preloadCallbacks: PreloadCallbacks
 }): Promise<FlightDataPath[]> {
@@ -154,7 +154,7 @@ export async function walkTreeWithFlightRouterState({
           // This is intentionally not "rootLayoutIncludedAtThisLevelOrAbove" as createComponentTree starts at the current level and does a check for "rootLayoutAtThisLevel" too.
           rootLayoutIncluded,
           asNotFound,
-          getMetadataReady,
+          metadataOutlet,
           preloadCallbacks,
         }
       )
@@ -215,7 +215,7 @@ export async function walkTreeWithFlightRouterState({
           injectedFontPreloadTags: injectedFontPreloadTagsWithCurrentLayout,
           rootLayoutIncluded: rootLayoutIncludedAtThisLevelOrAbove,
           asNotFound,
-          getMetadataReady,
+          metadataOutlet,
           preloadCallbacks,
         })