fix: build stats should properly report root page size in appDir (#51854

) ### What? `next build` is incorrectly reporting file size stats for the root path when using the app dir ### Why? In #50768, the denormalization logic was removed for appDir routes, but `pagesManifest` has the root page keyed by `/` while the actual path will be `/index`. ### How? This re-adds a simpler denormalize function that just replaces `/index` with `/`. semi-related: #42819 Fixes #51692 link NEXT-1306 --------- Co-authored-by: JJ Kasper <jj@jjsweb.site> Co-authored-by: Jiachi Liu <inbox@huozhi.im>
vercel · Jun 27, 2023 · aec75c2 · aec75c2
1 parent d06bc5e
commit aec75c2
Show file tree

Hide file tree

Showing 7 changed files with 108 additions and 5 deletions.
diff --git a/packages/next/src/build/utils.ts b/packages/next/src/build/utils.ts
@@ -65,6 +65,7 @@ import { patchFetch } from '../server/lib/patch-fetch'
 import { nodeFs } from '../server/lib/node-fs-methods'
 import * as ciEnvironment from '../telemetry/ci-info'
 import { normalizeAppPath } from '../shared/lib/router/utils/app-paths'
+import { denormalizeAppPagePath } from '../shared/lib/page-path/denormalize-app-path'
 
 export type ROUTER_TYPE = 'pages' | 'app'
 
@@ -793,11 +794,10 @@ export async function getJsPageSizeInKb(
     throw new Error('expected "app" manifest data with an "app" pageType')
   }
 
-  // Denormalization is not needed for "app" dir, as we normalize the keys of appBuildManifest instead
-  let pagePath = page
-  if (routerType === 'pages') {
-    pagePath = denormalizePagePath(page)
-  }
+  const pagePath =
+    routerType === 'pages'
+      ? denormalizePagePath(page)
+      : denormalizeAppPagePath(page)
 
   const fnFilterJs = (entry: string) => entry.endsWith('.js')
 

diff --git a/packages/next/src/shared/lib/page-path/denormalize-app-path.ts b/packages/next/src/shared/lib/page-path/denormalize-app-path.ts
@@ -0,0 +1,8 @@
+export function denormalizeAppPagePath(page: string): string {
+  // `/` is normalized to `/index`
+  if (page === '/index') {
+    return '/'
+  }
+
+  return page
+}
diff --git a/test/e2e/app-dir/build-size/app/foo/page.tsx b/test/e2e/app-dir/build-size/app/foo/page.tsx
@@ -0,0 +1,14 @@
+'use client'
+import React from 'react'
+import styles from '../../styles/index.module.css'
+
+export default function Page() {
+  return (
+    <>
+      <p id="hello" className={styles.content}>
+        hello from /foo
+      </p>
+      <p id="react-version">{React.version}</p>
+    </>
+  )
+}
diff --git a/test/e2e/app-dir/build-size/app/layout.tsx b/test/e2e/app-dir/build-size/app/layout.tsx
@@ -0,0 +1,16 @@
+export const metadata = {
+  title: 'Next.js',
+  description: 'Generated by Next.js',
+}
+
+export default function RootLayout({
+  children,
+}: {
+  children: React.ReactNode
+}) {
+  return (
+    <html lang="en">
+      <body>{children}</body>
+    </html>
+  )
+}
diff --git a/test/e2e/app-dir/build-size/app/page.tsx b/test/e2e/app-dir/build-size/app/page.tsx
@@ -0,0 +1,13 @@
+import React from 'react'
+import styles from '../styles/index.module.css'
+
+export default function Page() {
+  return (
+    <>
+      <p id="hello" className={styles.content}>
+        hello from /
+      </p>
+      <p id="react-version">{React.version}</p>
+    </>
+  )
+}
diff --git a/test/e2e/app-dir/build-size/index.test.ts b/test/e2e/app-dir/build-size/index.test.ts
@@ -0,0 +1,49 @@
+import { createNextDescribe } from 'e2e-utils'
+
+createNextDescribe(
+  'app-dir build size',
+  {
+    files: __dirname,
+    skipDeployment: true,
+  },
+  ({ next, isNextStart }) => {
+    if (isNextStart) {
+      it('should have correct size in build output', async () => {
+        const regex = /(\S+)\s+(\d+\s\w+)\s+(\d+\.\d+\s\w+)/g
+        const matches = [...next.cliOutput.matchAll(regex)]
+
+        const result = matches.reduce((acc, match) => {
+          const [, path, size, firstLoadJS] = match
+
+          acc[path] = { size, firstLoadJS }
+          return acc
+        }, {})
+
+        // convert pretty-bytes format into bytes so we can compare units
+        const sizeToBytes = (size: string) => {
+          const units = ['B', 'kB', 'MB', 'GB', 'TB', 'PB', 'EB', 'ZB', 'YB']
+          const [value, unit] = size.split(' ')
+          const exp = units.indexOf(unit)
+          return parseFloat(value) * Math.pow(1024, exp)
+        }
+
+        const index = result['/']
+        const foo = result['/foo']
+
+        // index route has a page, so it should not be 0
+        expect(sizeToBytes(index.size)).toBeGreaterThan(0)
+        expect(sizeToBytes(index.firstLoadJS)).toBeGreaterThan(0)
+
+        // foo route has a page, so it should not be 0
+        expect(sizeToBytes(foo.size)).toBeGreaterThan(0)
+        expect(sizeToBytes(foo.firstLoadJS)).toBeGreaterThan(0)
+
+        // foo is a client component, so it should be larger than index
+        expect(sizeToBytes(foo.size)).toBeGreaterThan(sizeToBytes(index.size))
+      })
+    } else {
+      it('should skip next dev for now', () => {})
+      return
+    }
+  }
+)
diff --git a/test/e2e/app-dir/build-size/styles/index.module.css b/test/e2e/app-dir/build-size/styles/index.module.css
@@ -0,0 +1,3 @@
+.content {
+  color: blueviolet;
+}