External vercel og for nodejs runtime (#48844)

### Why Default font file of `@vercel/og` is not loaded, because the og package is bundled by webpack and we should external it so that `fs.readFileSync` is bundled and manged that can't be traced by nft. ### How This PR externals `@vercel/og` so that they don't need to be bundled and files can be properly traced Closes NEXT-1047 Fixes #48704
vercel · Apr 26, 2023 · 98e0c4a · 98e0c4a
1 parent 219d1d4
commit 98e0c4a
Show file tree

Hide file tree

Showing 4 changed files with 126 additions and 96 deletions.
diff --git a/packages/next/src/build/webpack-config.ts b/packages/next/src/build/webpack-config.ts
@@ -1286,7 +1286,9 @@ export default async function getBaseWebpackConfig(
       // so that the DefinePlugin can inject process.env values.
 
       // Treat next internals as non-external for server layer
-      if (layer === WEBPACK_LAYERS.server) return
+      if (layer === WEBPACK_LAYERS.server) {
+        return
+      }
 
       const isNextExternal =
         /next[/\\]dist[/\\](esm[\\/])?(shared|server)[/\\](?!lib[/\\](router[/\\]router|dynamic|app-dynamic|lazy-dynamic|head[^-]))/.test(
@@ -1332,6 +1334,15 @@ export default async function getBaseWebpackConfig(
       resolveResult.res = require.resolve(request)
     }
 
+    // Don't bundle @vercel/og nodejs bundle for nodejs runtime
+    // TODO-APP: bundle route.js with different layer that externals common node_module deps
+    if (
+      layer === WEBPACK_LAYERS.server &&
+      request === 'next/dist/compiled/@vercel/og/index.node.js'
+    ) {
+      return `module ${request}`
+    }
+
     const { res, isEsm } = resolveResult
 
     // If the request cannot be resolved we need to have
@@ -1413,6 +1424,7 @@ export default async function getBaseWebpackConfig(
       if (layer === WEBPACK_LAYERS.server) {
         // All packages should be bundled for the server layer if they're not opted out.
         // This option takes priority over the transpilePackages option.
+
         if (optOutBundlingPackageRegex.test(res)) {
           return `${externalType} ${request}`
         }

diff --git a/packages/next/src/compiled/@vercel/og/index.node.js b/packages/next/src/compiled/@vercel/og/index.node.js
@@ -18145,4 +18145,4 @@ export {
  * Copyright © 2015 Gilmore Davidson under the MIT license: http://gilmoreorless.mit-license.org/
  */
 /*! Copyright Twitter Inc. and other contributors. Licensed under MIT */
-//# sourceMappingURL=index.node.js.map
+//# sourceMappingURL=index.node.js.map
diff --git a/packages/next/src/server/web/spec-extension/image-response.ts b/packages/next/src/server/web/spec-extension/image-response.ts
@@ -17,8 +17,8 @@ export class ImageResponse {
             (
               await import(
                 process.env.NEXT_RUNTIME === 'edge'
-                  ? 'next/dist/compiled/@vercel/og/index.edge'
-                  : 'next/dist/compiled/@vercel/og/index.node'
+                  ? 'next/dist/compiled/@vercel/og/index.edge.js'
+                  : 'next/dist/compiled/@vercel/og/index.node.js'
               )
             ).ImageResponse
           const imageResponse = new OGImageResponse(...args) as Response

diff --git a/test/e2e/app-dir/metadata-dynamic-routes/index.test.ts b/test/e2e/app-dir/metadata-dynamic-routes/index.test.ts
@@ -13,7 +13,6 @@ createNextDescribe(
   'app dir - metadata dynamic routes',
   {
     files: __dirname,
-    skipDeployment: true,
   },
   ({ next, isNextDev, isNextStart, isNextDeploy }) => {
     describe('text routes', () => {
@@ -109,97 +108,102 @@ createNextDescribe(
         )
       })
 
-      it('should support generate multi images with generateImageMetadata', async () => {
-        const $ = await next.render$('/dynamic/big')
-        const iconUrls = $('link[rel="icon"]')
-          .toArray()
-          .map((el) => {
-            return {
-              href: $(el).attr('href').split('?')[0],
-              sizes: $(el).attr('sizes'),
-              type: $(el).attr('type'),
-            }
-          })
-        // slug is id param from generateImageMetadata
-        expect(iconUrls).toMatchObject([
-          {
-            href: '/dynamic/big/icon-48jo90/small',
-            sizes: '48x48',
-            type: 'image/png',
-          },
-          {
-            href: '/dynamic/big/icon-48jo90/medium',
-            sizes: '72x72',
-            type: 'image/png',
-          },
-        ])
-
-        const appleTouchIconUrls = $('link[rel="apple-touch-icon"]')
-          .toArray()
-          .map((el) => {
-            return {
-              href: $(el).attr('href').split('?')[0],
-              sizes: $(el).attr('sizes'),
-              type: $(el).attr('type'),
-            }
-          })
-        // slug is index by default
-        expect(appleTouchIconUrls).toEqual([
-          {
-            href: '/dynamic/big/apple-icon-48jo90/0',
-            sizes: '48x48',
-            type: 'image/png',
-          },
-          {
-            href: '/dynamic/big/apple-icon-48jo90/1',
-            sizes: '64x64',
-            type: 'image/png',
-          },
-        ])
-      })
+      if (!isNextDeploy) {
+        // TODO-APP: investigate the dynamic routes failing in deployment tests
+        it('should support generate multi images with generateImageMetadata', async () => {
+          const $ = await next.render$('/dynamic/big')
+          const iconUrls = $('link[rel="icon"]')
+            .toArray()
+            .map((el) => {
+              return {
+                href: $(el).attr('href').split('?')[0],
+                sizes: $(el).attr('sizes'),
+                type: $(el).attr('type'),
+              }
+            })
+          // slug is id param from generateImageMetadata
+          expect(iconUrls).toMatchObject([
+            {
+              href: '/dynamic/big/icon-48jo90/small',
+              sizes: '48x48',
+              type: 'image/png',
+            },
+            {
+              href: '/dynamic/big/icon-48jo90/medium',
+              sizes: '72x72',
+              type: 'image/png',
+            },
+          ])
+
+          const appleTouchIconUrls = $('link[rel="apple-touch-icon"]')
+            .toArray()
+            .map((el) => {
+              return {
+                href: $(el).attr('href').split('?')[0],
+                sizes: $(el).attr('sizes'),
+                type: $(el).attr('type'),
+              }
+            })
+          // slug is index by default
+          expect(appleTouchIconUrls).toEqual([
+            {
+              href: '/dynamic/big/apple-icon-48jo90/0',
+              sizes: '48x48',
+              type: 'image/png',
+            },
+            {
+              href: '/dynamic/big/apple-icon-48jo90/1',
+              sizes: '64x64',
+              type: 'image/png',
+            },
+          ])
+        })
 
-      it('should support generate multi sitemaps with generateSitemaps', async () => {
-        const ids = [0, 1, 2, 3]
-        function fetchSitemap(id) {
-          return next
-            .fetch(`/dynamic/small/sitemap.xml/${id}`)
-            .then((res) => res.text())
-        }
-
-        for (const id of ids) {
-          const text = await fetchSitemap(id)
-          expect(text).toContain(`<loc>https://example.com/dynamic/${id}</loc>`)
-        }
-      })
+        it('should support generate multi sitemaps with generateSitemaps', async () => {
+          const ids = [0, 1, 2, 3]
+          function fetchSitemap(id) {
+            return next
+              .fetch(`/dynamic/small/sitemap.xml/${id}`)
+              .then((res) => res.text())
+          }
+
+          for (const id of ids) {
+            const text = await fetchSitemap(id)
+            expect(text).toContain(
+              `<loc>https://example.com/dynamic/${id}</loc>`
+            )
+          }
+        })
 
-      it('should fill params into dynamic routes url of metadata images', async () => {
-        const $ = await next.render$('/dynamic/big')
-        const ogImageUrl = $('meta[property="og:image"]').attr('content')
-        expect(ogImageUrl).toMatch(hashRegex)
-        expect(ogImageUrl).toMatch('/dynamic/big/opengraph-image')
-        // should already normalize the parallel routes segment to url
-        expect(ogImageUrl).not.toContain('(group)')
-      })
+        it('should fill params into dynamic routes url of metadata images', async () => {
+          const $ = await next.render$('/dynamic/big')
+          const ogImageUrl = $('meta[property="og:image"]').attr('content')
+          expect(ogImageUrl).toMatch(hashRegex)
+          expect(ogImageUrl).toMatch('/dynamic/big/opengraph-image')
+          // should already normalize the parallel routes segment to url
+          expect(ogImageUrl).not.toContain('(group)')
+        })
 
-      it('should support params as argument in dynamic routes', async () => {
-        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(smallOgUrl.pathname)
-        ).buffer()
-
-        const sizeBig = imageSize(bufferBig)
-        const sizeSmall = imageSize(bufferSmall)
-        expect([sizeBig.width, sizeBig.height]).toEqual([1200, 630])
-        expect([sizeSmall.width, sizeSmall.height]).toEqual([600, 315])
-      })
+        it('should support params as argument in dynamic routes', async () => {
+          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(smallOgUrl.pathname)
+          ).buffer()
+
+          const sizeBig = imageSize(bufferBig)
+          const sizeSmall = imageSize(bufferSmall)
+          expect([sizeBig.width, sizeBig.height]).toEqual([1200, 630])
+          expect([sizeSmall.width, sizeSmall.height]).toEqual([600, 315])
+        })
+      }
 
       it('should fill params into routes groups url of static images', async () => {
         const $ = await next.render$('/static')
@@ -294,8 +298,8 @@ createNextDescribe(
       let twitterImageUrlPattern
       if (isNextDeploy) {
         // absolute urls
-        ogImageUrlPattern = /https:\/\/\w+.vercel.app\/opengraph-image\?/
-        twitterImageUrlPattern = /https:\/\/\w+.vercel.app\/twitter-image\?/
+        ogImageUrlPattern = /https:\/\/[\w-]+.vercel.app\/opengraph-image\?/
+        twitterImageUrlPattern = /https:\/\/[\w-]+.vercel.app\/twitter-image\?/
       } else if (isNextStart) {
         // configured metadataBase for next start
         ogImageUrlPattern = /https:\/\/mydomain.com\/opengraph-image\?/
@@ -325,7 +329,7 @@ createNextDescribe(
 
       if (isNextDeploy) {
         expect(twitterImage).toMatch(
-          /https:\/\/\w+.vercel.app\/metadata-base\/unset\/twitter-image\.png/
+          /https:\/\/[\w-]+.vercel.app\/metadata-base\/unset\/twitter-image\.png/
         )
       } else {
         expect(twitterImage).toMatch(
@@ -347,6 +351,20 @@ createNextDescribe(
           /\/\(group\)\/twitter-image-\w{6}\/\[\[\.\.\.__metadata_id__\]\]\/route/
         )
       })
+
+      it('should include default og font files in file trace', async () => {
+        const fileTrace = JSON.parse(
+          await next.readFile(
+            '.next/server/app/opengraph-image/[[...__metadata_id__]]/route.js.nft.json'
+          )
+        )
+
+        // @vercel/og default font should be traced
+        const isTraced = fileTrace.files.some((filePath) =>
+          filePath.includes('/noto-sans-v27-latin-regular.ttf')
+        )
+        expect(isTraced).toBe(true)
+      })
     }
   }
 )