fix: handle review comments

Author: wyattjoh

packages/next/src/build/index.ts

@@ -3480,7 +3480,9 @@ export default async function build(
                     route.fallbackRouteParams &&
                     route.fallbackRouteParams.length > 0 &&
                     // If all the fallback route params are parallel route
-                    // params, then we still consider it a static route.
+                    // params, then we still consider it a static route. This is
+                    // required to ensure that the changes are backwards
+                    // compatible with the previous builder implementations.
                     !route.fallbackRouteParams.every(
                       (param) => param.isParallelRouteParam
                     )

packages/next/src/build/static-paths/app.ts

@@ -19,7 +19,6 @@ import type { IncrementalCache } from '../../server/lib/incremental-cache'
 import {
   normalizePathname,
   encodeParam,
-  filterNonParallelFallbackRouteParams,
   createFallbackRouteParam,
 } from './utils'
 import escapePathDelimiters from '../../shared/lib/router/utils/escape-path-delimiters'
@@ -504,7 +503,9 @@ export function assignErrorIfEmpty(
           // calculation because parallel route params won't contribute to
           // the request pathname.
           r.fallbackRouteParams
-            ? filterNonParallelFallbackRouteParams(r.fallbackRouteParams).length
+            ? r.fallbackRouteParams.filter(
+                (param) => !param.isParallelRouteParam
+              ).length
             : 0
         )
       }
@@ -529,8 +530,9 @@ export function assignErrorIfEmpty(
             // We only consider non-parallel fallback route params for this
             // calculation because parallel route params won't contribute to
             // the request pathname.
-            filterNonParallelFallbackRouteParams(route.fallbackRouteParams)
-              .length > minFallbacks)
+            route.fallbackRouteParams.filter(
+              (param) => !param.isParallelRouteParam
+            ).length > minFallbacks)
         ) {
           route.throwOnEmptyStaticShell = false // Should not throw on empty static shell.
         } else {
@@ -921,9 +923,8 @@ export async function buildAppStaticPaths({
       const fallbackRootParams: string[] = []
       // Only add the param to the fallback root params if it's not a
       // parallel route param. They won't contribute to the request pathname.
-      for (const { paramName } of filterNonParallelFallbackRouteParams(
-        fallbackRouteParams
-      )) {
+      for (const { paramName, isParallelRouteParam } of fallbackRouteParams) {
+        if (isParallelRouteParam) continue
         if (rootParamSet.has(paramName)) {
           fallbackRootParams.push(paramName)
         }

packages/next/src/build/static-paths/utils.ts

@@ -45,15 +45,3 @@ export function createFallbackRouteParam(
 ): FallbackRouteParam {
   return { paramName, isParallelRouteParam }
 }
-
-/**
- * Filters out all the parallel route params from the fallback route params.
- *
- * @param fallbackRouteParams - The fallback route params to filter.
- * @returns The filtered fallback route params.
- */
-export function filterNonParallelFallbackRouteParams(
-  fallbackRouteParams: readonly FallbackRouteParam[]
-): readonly FallbackRouteParam[] {
-  return fallbackRouteParams.filter((param) => !param.isParallelRouteParam)
-}

packages/next/src/client/components/navigation-untracked.ts

@@ -21,6 +21,9 @@ function hasFallbackRouteParams(): boolean {
       case 'prerender-client':
       case 'prerender-ppr':
         const fallbackParams = workUnitStore.fallbackRouteParams
+        // We only consider the non-parallel fallback route params for this
+        // check because parallel route params won't contribute to the request
+        // pathname.
         return fallbackParams ? fallbackParams.sizes.route > 0 : false
       case 'prerender-legacy':
       case 'request':

packages/next/src/export/routes/app-page.ts

@@ -139,6 +139,8 @@ export async function exportAppPage(
       // means that the route has unknown route params.
       if (
         !fallbackRouteParams ||
+        // We only consider the non-parallel fallback route params to maintain
+        // backwards compatibility with the previous implementation.
         fallbackRouteParams.sizes.route === 0 ||
         renderOpts.experimental.clientParamParsing
       ) {

packages/next/src/server/app-render/app-render.tsx

@@ -3910,7 +3910,11 @@ async function prerenderToStream(
       // move the fallback route params out of the flight router state, we need
       // to always perform a dynamic resume after the static prerender.
       const hasFallbackRouteParams =
-        fallbackRouteParams && fallbackRouteParams.sizes.route > 0
+        // We consider both the route and parallel fallback route params for
+        // this check because we're determining if we can emit a completely
+        // static HTML prerender. If any of the route params are unknown then
+        // the prerender will be dynamic.
+        fallbackRouteParams && fallbackRouteParams.sizes.total > 0
 
       if (serverIsDynamic || hasFallbackRouteParams) {
         // Dynamic case

packages/next/src/server/app-render/dynamic-rendering.ts

@@ -595,7 +595,10 @@ export function useDynamicRouteParams(expression: string) {
       case 'prerender-client':
       case 'prerender': {
         const fallbackParams = workUnitStore.fallbackRouteParams
-        if (fallbackParams && fallbackParams.sizes.route > 0) {
+        // We consider both the route and parallel fallback route params for
+        // this check because `useParams()` will also return the parallel
+        // fallback route params.
+        if (fallbackParams && fallbackParams.sizes.total > 0) {
           // We are in a prerender with cacheComponents semantics. We are going to
           // hang here and never resolve. This will cause the currently
           // rendering component to effectively be a dynamic hole.
@@ -611,7 +614,10 @@ export function useDynamicRouteParams(expression: string) {
       }
       case 'prerender-ppr': {
         const fallbackParams = workUnitStore.fallbackRouteParams
-        if (fallbackParams && fallbackParams.sizes.route > 0) {
+        // We consider both the route and parallel fallback route params for
+        // this check because `useParams()` will also return the parallel
+        // fallback route params.
+        if (fallbackParams && fallbackParams.sizes.total > 0) {
           return postponeWithTracking(
             workStore.route,
             expression,

packages/next/src/server/app-render/postponed-state.ts

@@ -80,7 +80,11 @@ export async function getDynamicHTMLPostponedState(
   const data: DynamicHTMLPostponedState['data'] = [preludeState, postponed]
   const dataString = JSON.stringify(data)
 
-  if (!fallbackRouteParams || fallbackRouteParams.sizes.route === 0) {
+  // If there are no fallback route params, we can just serialize the postponed
+  // state as is. We consider both the route and parallel fallback route params
+  // for this check because they both contribute to the outputted postponed
+  // state.
+  if (!fallbackRouteParams || fallbackRouteParams.sizes.total === 0) {
     // Serialized as `<postponedString.length>:<postponedString><renderResumeDataCache>`
     return `${dataString.length}:${dataString}${await stringifyResumeDataCache(
       createRenderResumeDataCache(resumeDataCache)

packages/next/src/server/lib/implicit-tags.ts

@@ -80,6 +80,9 @@ export async function getImplicitTags(
   fallbackRouteParams: null | OpaqueFallbackRouteParams
 ): Promise<ImplicitTags> {
   const tags: string[] = []
+
+  // As we're using this to determine if the route pathname is dynamic, we only
+  // consider the route fallback route params for this check.
   const hasFallbackRouteParams =
     fallbackRouteParams && fallbackRouteParams.sizes.route > 0
 

packages/next/src/server/request/fallback-params.ts

@@ -36,6 +36,11 @@ export type OpaqueFallbackRouteParams = {
      * the route should be considered dynamic.
      */
     readonly parallel: number
+
+    /**
+     * The total number of fallback route params.
+     */
+    readonly total: number
   }
 
   /**
@@ -83,14 +88,15 @@ export function createOpaqueFallbackRouteParams(
   // be also be unique.
   const uniqueID = Math.random().toString(16).slice(2)
 
-  const sizes = { route: 0, parallel: 0 }
+  const sizes = { route: 0, parallel: 0, total: 0 }
   const keys = new Map<string, string>()
 
   for (const { paramName, isParallelRouteParam } of fallbackRouteParams) {
     // We need to track the sizes of the fallback route params to determine if
     // the render should be halted during static generation.
     if (isParallelRouteParam) sizes.parallel++
     else sizes.route++
+    sizes.total++
 
     // Generate a unique key for the fallback route param, if this key is found
     // in the static output, it represents a bug in cache components.
@@ -102,10 +108,7 @@ export function createOpaqueFallbackRouteParams(
     has: keys.has.bind(keys),
     get: keys.get.bind(keys),
     *[Symbol.iterator](): IterableIterator<[string, string]> {
-      for (const { paramName, isParallelRouteParam } of fallbackRouteParams) {
-        // We only want to include the route segments, not the parallel route
-        // segments.
-        if (isParallelRouteParam) continue
+      for (const { paramName } of fallbackRouteParams) {
         yield [paramName, keys.get(paramName)!]
       }
     },