Create unique route path for og under group routes (#47985)

### What

When using dynamic metadata image rouets (such as `opengraph-image.js`)
under group routes, the generated urls were still normalized. In this
case it might have conflicts with those ones not under group routes. For
instance `app/(post)/opengraph-image.js` could have same url with
`/app/opengraph-image.js`. In reality we want them to be different
route, unlike layout or pages.

### How

So when we found `()` or `@` signs from the metadata image urls, we'll
generate a unqiue suffix (`-\d{6}`) and append to the generated url. So
they can be isolated from the ones are not under special convention
routes.

Closes NEXT-937

packages/next/src/build/webpack/loaders/metadata/discover.ts

@@ -6,6 +6,7 @@ import type {
 import path from 'path'
 import { stringify } from 'querystring'
 import { STATIC_METADATA_IMAGES } from '../../../../lib/metadata/is-metadata-route'
+import { normalizeAppPath } from '../../../../shared/lib/router/utils/app-paths'
 
 const METADATA_TYPE = 'metadata'
 
@@ -54,13 +55,13 @@ async function enumMetadataFiles(
 export async function createStaticMetadataFromRoute(
   resolvedDir: string,
   {
-    route,
+    segment,
     resolvePath,
     isRootLayer,
     loaderContext,
     pageExtensions,
   }: {
-    route: string
+    segment: string
     resolvePath: (pathname: string) => Promise<string>
     isRootLayer: boolean
     loaderContext: webpack.LoaderContext<any>
@@ -98,7 +99,8 @@ export async function createStaticMetadataFromRoute(
         const imageModuleImportSource = `next-metadata-image-loader?${stringify(
           {
             type,
-            route,
+            segment,
+            route: normalizeAppPath(segment),
             pageExtensions,
           }
         )}!${filepath}${METADATA_RESOURCE_QUERY}`

packages/next/src/build/webpack/loaders/next-app-loader.ts

@@ -20,7 +20,6 @@ import { isAppRouteRoute } from '../../../lib/is-app-route-route'
 import { isMetadataRoute } from '../../../lib/metadata/is-metadata-route'
 import { NextConfig } from '../../../server/config-shared'
 import { AppPathnameNormalizer } from '../../../server/future/normalizers/built/app/app-pathname-normalizer'
-import { normalizeAppPath } from '../../../shared/lib/router/utils/app-paths'
 
 export type AppLoaderOptions = {
   name: string
@@ -255,7 +254,7 @@ async function createTreeCodeFromPath(
 
       if (resolvedRouteDir) {
         metadata = await createStaticMetadataFromRoute(resolvedRouteDir, {
-          route: normalizeAppPath(segmentPath),
+          segment: segmentPath,
           resolvePath,
           isRootLayer,
           loaderContext,
@@ -348,7 +347,7 @@ async function createTreeCodeFromPath(
               )}), ${JSON.stringify(filePath)}],`
             })
             .join('\n')}
-          ${definedFilePaths.length ? createMetadataExportsCode(metadata) : ''}
+          ${createMetadataExportsCode(metadata)}
         }
       ]`
     }

packages/next/src/build/webpack/loaders/next-metadata-image-loader.ts

@@ -13,13 +13,14 @@ import { imageExtMimeTypeMap } from '../../../lib/mime-type'
 
 interface Options {
   route: string
+  segment: string
   type: PossibleImageFileNameConvention
   pageExtensions: string[]
 }
 
 async function nextMetadataImageLoader(this: any, content: Buffer) {
   const options: Options = this.getOptions()
-  const { type, route, pageExtensions } = options
+  const { type, route, segment, pageExtensions } = options
   const numericSizes = type === 'twitter' || type === 'openGraph'
   const { resourcePath, rootContext: context } = this
   const { name: fileNameBase, ext } = path.parse(resourcePath)
@@ -44,9 +45,8 @@ async function nextMetadataImageLoader(this: any, content: Buffer) {
   )
 
   const isDynamicResource = pageExtensions.includes(extension)
-  const pageRoute =
-    (isDynamicResource ? fileNameBase : interpolatedName) +
-    (contentHash ? '?' + contentHash : '')
+  const pageRoute = isDynamicResource ? fileNameBase : interpolatedName
+  const hashQuery = contentHash ? '?' + contentHash : ''
 
   if (isDynamicResource) {
     // re-export and spread as `exportedImageData` to avoid non-exported error
@@ -55,17 +55,23 @@ async function nextMetadataImageLoader(this: any, content: Buffer) {
     import * as exported from ${JSON.stringify(resourcePath)}
     import { interpolateDynamicPath } from 'next/dist/server/server-utils'
     import { getNamedRouteRegex } from 'next/dist/shared/lib/router/utils/route-regex'
+    import { getMetadataRouteSuffix } from 'next/dist/lib/metadata/get-metadata-route'
 
     const exportedImageData = { ...exported }
     export default (props) => {
       const pathname = ${JSON.stringify(route)}
       const routeRegex = getNamedRouteRegex(pathname, false)
+      const segment = ${JSON.stringify(segment)}
       const route = interpolateDynamicPath(pathname, props.params, routeRegex)
+      const suffix = getMetadataRouteSuffix(segment)
+      const routeSuffix = suffix ? \`-\${suffix}\` : ''
 
       const imageData = {
         alt: exportedImageData.alt,
-        type: exportedImageData.contentType,
-        url: path.join(route, ${JSON.stringify(pageRoute)}),
+        type: exportedImageData.contentType || 'image/png',
+        url: path.join(route, ${JSON.stringify(
+          pageRoute
+        )} + routeSuffix + ${JSON.stringify(hashQuery)}),
       }
       const { size } = exportedImageData
       if (size) {
@@ -118,7 +124,9 @@ async function nextMetadataImageLoader(this: any, content: Buffer) {
 
     return {
       ...imageData,
-      url: path.join(route, ${JSON.stringify(pageRoute)}),
+      url: path.join(route, ${JSON.stringify(pageRoute)} + ${JSON.stringify(
+    hashQuery
+  )}),
     }
   }`
 }

packages/next/src/lib/metadata/get-metadata-route.ts

@@ -1,4 +1,23 @@
 import { isMetadataRoute } from './is-metadata-route'
+import path from '../../shared/lib/isomorphic/path'
+import { djb2Hash } from '../../shared/lib/hash'
+
+/*
+ * If there's special convention like (...) or @ in the page path,
+ * Give it a unique hash suffix to avoid conflicts
+ *
+ * e.g.
+ * /app/open-graph.tsx -> /open-graph/route
+ * /app/(post)/open-graph.tsx -> /open-graph/route-123456
+ */
+export function getMetadataRouteSuffix(page: string) {
+  let suffix = ''
+
+  if ((page.includes('(') && page.includes(')')) || page.includes('@')) {
+    suffix = djb2Hash(page).toString().slice(0, 6)
+  }
+  return suffix
+}
 
 /**
  * Map metadata page key to the corresponding route
@@ -12,8 +31,10 @@ import { isMetadataRoute } from './is-metadata-route'
 export function normalizeMetadataRoute(page: string) {
   let route = page
   if (isMetadataRoute(page)) {
-    // TODO-METADATA: add dynamic routes for metadata images.
-    // Better to move the extension appending to early phase.
+    // Remove the file extension, e.g. /route-path/robots.txt -> /route-path
+    const pathnamePrefix = page.slice(0, -(path.basename(page).length + 1))
+    const suffix = getMetadataRouteSuffix(pathnamePrefix)
+
     if (route === '/sitemap') {
       route += '.xml'
     }
@@ -26,7 +47,7 @@ export function normalizeMetadataRoute(page: string) {
     // Support both /<metadata-route.ext> and custom routes /<metadata-route>/route.ts.
     // If it's a metadata file route, we need to append /route to the page.
     if (!route.endsWith('/route')) {
-      route = `${route}/route`
+      route = `${route}${suffix ? `-${suffix}` : ''}/route`
     }
   }
   return route

packages/next/src/lib/metadata/resolve-metadata.ts

@@ -257,13 +257,13 @@ async function resolveStaticMetadata(components: ComponentsType, props: any) {
 // [layout.metadata, static files metadata] -> ... -> [page.metadata, static files metadata]
 export async function collectMetadata({
   loaderTree,
+  metadataItems: array,
   props,
-  array,
   route,
 }: {
   loaderTree: LoaderTree
+  metadataItems: MetadataItems
   props: any
-  array: MetadataItems
   route: string
 }) {
   const [mod, modType] = await getLayoutOrPageModule(loaderTree)
@@ -289,7 +289,6 @@ export async function accumulateMetadata(
   options: MetadataAccumulationOptions
 ): Promise<ResolvedMetadata> {
   const resolvedMetadata = createDefaultMetadata()
-
   const resolvers: ((value: ResolvedMetadata) => void)[] = []
   const generateMetadataResults: (Metadata | Promise<Metadata>)[] = []
 

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

@@ -384,8 +384,8 @@ export async function renderToHTMLOrFlight(
 
       await collectMetadata({
         loaderTree: tree,
+        metadataItems,
         props: layerProps,
-        array: metadataItems,
         route: currentTreePrefix
           // __PAGE__ shouldn't be shown in a route
           .filter((s) => s !== PAGE_SEGMENT_KEY)
@@ -396,8 +396,8 @@ export async function renderToHTMLOrFlight(
         const childTree = parallelRoutes[key]
         await resolveMetadata({
           tree: childTree,
-          parentParams: currentParams,
           metadataItems,
+          parentParams: currentParams,
           treePrefix: currentTreePrefix,
         })
       }

packages/next/src/shared/lib/hash.ts

@@ -0,0 +1,9 @@
+// http://www.cse.yorku.ca/~oz/hash.html
+export function djb2Hash(str: string) {
+  let hash = 5381
+  for (let i = 0; i < str.length; i++) {
+    const char = str.charCodeAt(i)
+    hash = (hash << 5) + hash + char
+  }
+  return Math.abs(hash)
+}

test/e2e/app-dir/metadata-dynamic-routes/app/(group)/blog/page.tsx

@@ -0,0 +1,3 @@
+export default function page() {
+  return <>{`(group)blog`}</>
+}

test/e2e/app-dir/metadata-dynamic-routes/app/(group)/layout.tsx

@@ -0,0 +1,14 @@
+export default function layout({ children }) {
+  return (
+    <html>
+      <head></head>
+      <body>
+        <div className="group">{children}</div>
+      </body>
+    </html>
+  )
+}
+
+export const metadata = {
+  title: 'Group Title',
+}

test/e2e/app-dir/metadata-dynamic-routes/app/(group)/opengraph-image.tsx

@@ -0,0 +1,22 @@
+import { ImageResponse } from 'next/server'
+
+export default function og() {
+  return new ImageResponse(
+    (
+      <div
+        style={{
+          width: '100%',
+          height: '100%',
+          display: 'flex',
+          alignItems: 'center',
+          justifyContent: 'center',
+          fontSize: 128,
+          color: '#fff',
+          background: '#000',
+        }}
+      >
+        group route og
+      </div>
+    )
+  )
+}

test/e2e/app-dir/metadata-dynamic-routes/index.test.ts

@@ -119,11 +119,17 @@ createNextDescribe(
       })
 
       it('should support params as argument in dynamic routes', async () => {
-        const bufferBig = await (
-          await next.fetch('/dynamic/big/opengraph-image')
-        ).buffer()
+        const big$ = await next.render$('/dynamic/big')
+        const small$ = await next.render$('/dynamic/small')
+        const bigOgUrl = new URL(
+          big$('meta[property="og:image"]').attr('content')
+        )
+        const smallOgUrl = new URL(
+          small$('meta[property="og:image"]').attr('content')
+        )
+        const bufferBig = await (await next.fetch(bigOgUrl.pathname)).buffer()
         const bufferSmall = await (
-          await next.fetch('/dynamic/small/opengraph-image')
+          await next.fetch(smallOgUrl.pathname)
         ).buffer()
 
         const sizeBig = imageSize(bufferBig)
@@ -153,6 +159,18 @@ createNextDescribe(
       })
     })
 
+    it('should generate unique path for image routes under group routes', async () => {
+      const $ = await next.render$('/blog')
+      const ogImageUrl = $('meta[property="og:image"]').attr('content')
+      const ogImageUrlInstance = new URL(ogImageUrl)
+      const res = await next.fetch(ogImageUrlInstance.pathname)
+
+      // generate unique path with suffix for image routes under group routes
+      expect(ogImageUrl).toMatch(/opengraph-image-\d{6}\?/)
+      expect(ogImageUrl).toMatch(hashRegex)
+      expect(res.status).toBe(200)
+    })
+
     it('should inject dynamic metadata properly to head', async () => {
       const $ = await next.render$('/')
       const $icon = $('link[rel="icon"]')
@@ -182,14 +200,18 @@ createNextDescribe(
 
       if (isNextDeploy) {
         // absolute urls
-        expect(ogImageUrl).toMatch(/https:\/\/\w+.vercel.app\/opengraph-image/)
+        expect(ogImageUrl).toMatch(
+          /https:\/\/\w+.vercel.app\/opengraph-image\?/
+        )
         expect(twitterImageUrl).toMatch(
-          /https:\/\/\w+.vercel.app\/twitter-image/
+          /https:\/\/\w+.vercel.app\/twitter-image\?/
         )
       } else {
         // absolute urls
-        expect(ogImageUrl).toMatch(/http:\/\/localhost:\d+\/opengraph-image/)
-        expect(twitterImageUrl).toMatch(/http:\/\/localhost:\d+\/twitter-image/)
+        expect(ogImageUrl).toMatch(/http:\/\/localhost:\d+\/opengraph-image\?/)
+        expect(twitterImageUrl).toMatch(
+          /http:\/\/localhost:\d+\/twitter-image\?/
+        )
       }
       expect(ogImageUrl).toMatch(hashRegex)
       expect(twitterImageUrl).toMatch(hashRegex)