From 819243aac778daf916d66f5a6845c427fdd4f21c Mon Sep 17 00:00:00 2001 From: Luke Sandberg Date: Thu, 11 Sep 2025 17:08:24 -0700 Subject: [PATCH 1/3] Fix an issue where we include the size of css files in the 'First Load JS shared by all` instead report the js size --- packages/next/src/build/utils.ts | 35 +++++++++++++++++--------------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/packages/next/src/build/utils.ts b/packages/next/src/build/utils.ts index f564d658bab21..53bf87338970e 100644 --- a/packages/next/src/build/utils.ts +++ b/packages/next/src/build/utils.ts @@ -96,18 +96,14 @@ const print = console.log const RESERVED_PAGE = /^\/(_app|_error|_document|api(\/|$))/ const fileGzipStats: { [k: string]: Promise | undefined } = {} const fsStatGzip = (file: string) => { - const cached = fileGzipStats[file] - if (cached) return cached - return (fileGzipStats[file] = getGzipSize.file(file)) + return (fileGzipStats[file] ??= getGzipSize.file(file)) } const fileSize = async (file: string) => (await fs.stat(file)).size const fileStats: { [k: string]: Promise | undefined } = {} const fsStat = (file: string) => { - const cached = fileStats[file] - if (cached) return cached - return (fileStats[file] = fileSize(file)) + return (fileStats[file] ??= fileSize(file)) } export function unique(main: ReadonlyArray, sub: ReadonlyArray): T[] { @@ -139,7 +135,8 @@ function sum(a: ReadonlyArray): number { type ComputeFilesGroup = { files: ReadonlyArray size: { - total: number + totalJs: number + totalCss: number } } @@ -268,7 +265,11 @@ export async function computeFromManifest( const size = stats.get(f) if (typeof size === 'number') { - acc.size.total += size + if (f.endsWith('.js')) { + acc.size.totalJs += size + } else if (f.endsWith('.css')) { + acc.size.totalCss += size + } } return acc @@ -276,7 +277,8 @@ export async function computeFromManifest( { files: [] as string[], size: { - total: 0, + totalJs: 0, + totalCss: 0, }, } ) @@ -689,7 +691,7 @@ export async function printTreeView( } }) - const sharedFilesSize = stats.router[routerType]?.common.size.total + const sharedFilesJsSize = stats.router[routerType]?.common.size.totalJs const sharedFiles = process.env.__NEXT_PRIVATE_DETERMINISTIC_BUILD_OUTPUT ? [] @@ -697,15 +699,16 @@ export async function printTreeView( messages.push([ '+ First Load JS shared by all', - typeof sharedFilesSize === 'number' - ? getPrettySize(sharedFilesSize, { strong: true }) + typeof sharedFilesJsSize === 'number' + ? getPrettySize(sharedFilesJsSize, { strong: true }) : '', '', '', '', ]) const sharedCssFiles: string[] = [] - const sharedJsChunks = [ + // shared chunks include the js and then the css with sorted by name. + const sharedChunks = [ ...sharedFiles .filter((file) => { if (file.endsWith('.css')) { @@ -719,11 +722,11 @@ export async function printTreeView( ...sharedCssFiles.map((e) => e.replace(buildId, '')).sort(), ] - // if the some chunk are less than 10kb or we don't know the size, we only show the total size of the rest - const tenKbLimit = 10 * 1000 + // if some chunks are less than 10kb or we don't show it, we only show the total size of the rest + const tenKbLimit = 0 let restChunkSize = 0 let restChunkCount = 0 - sharedJsChunks.forEach((fileName, index, { length }) => { + sharedChunks.forEach((fileName, index, { length }) => { const innerSymbol = index + restChunkCount === length - 1 ? '└' : '├' const originalName = fileName.replace('', buildId) From e5faab3417f911a154853e5c6dfb0d83a4096537 Mon Sep 17 00:00:00 2001 From: Luke Sandberg Date: Thu, 11 Sep 2025 17:30:29 -0700 Subject: [PATCH 2/3] whups --- packages/next/src/build/utils.ts | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/packages/next/src/build/utils.ts b/packages/next/src/build/utils.ts index 53bf87338970e..74f2636c5ebf9 100644 --- a/packages/next/src/build/utils.ts +++ b/packages/next/src/build/utils.ts @@ -618,10 +618,7 @@ export async function printTreeView( const contSymbol = i === arr.length - 1 ? ' ' : '├' let routes: { route: string; duration: number; avgDuration?: number }[] - if ( - pageInfo.ssgPageDurations && - pageInfo.ssgPageDurations.some((d) => d > MIN_DURATION) - ) { + if (pageInfo.ssgPageDurations?.some((d) => d > MIN_DURATION)) { const previewPages = totalRoutes === 8 ? 8 : Math.min(totalRoutes, 7) const routesWithDuration = pageInfo.ssgPageRoutes .map((route, idx) => ({ @@ -723,7 +720,7 @@ export async function printTreeView( ] // if some chunks are less than 10kb or we don't show it, we only show the total size of the rest - const tenKbLimit = 0 + const tenKbLimit = 10 * 1000 let restChunkSize = 0 let restChunkCount = 0 sharedChunks.forEach((fileName, index, { length }) => { From 5f3d028b80f2a82f07f101938c564d9dc2140039 Mon Sep 17 00:00:00 2001 From: Luke Sandberg Date: Fri, 12 Sep 2025 16:43:10 -0700 Subject: [PATCH 3/3] Enhance the build-manifest plugin to account for the app dir structure Previously it contained entries for layouts and parallel routes, this confused the 'unique files' computation used to compute 'First Load JS' metrics. --- .../plugins/app-build-manifest-plugin.test.ts | 384 ++++++++++++++++++ .../plugins/app-build-manifest-plugin.ts | 147 ++++++- 2 files changed, 524 insertions(+), 7 deletions(-) create mode 100644 packages/next/src/build/webpack/plugins/app-build-manifest-plugin.test.ts diff --git a/packages/next/src/build/webpack/plugins/app-build-manifest-plugin.test.ts b/packages/next/src/build/webpack/plugins/app-build-manifest-plugin.test.ts new file mode 100644 index 0000000000000..5b17a3e70c514 --- /dev/null +++ b/packages/next/src/build/webpack/plugins/app-build-manifest-plugin.test.ts @@ -0,0 +1,384 @@ +// Mock webpack sources +jest.mock('next/dist/compiled/webpack/webpack', () => ({ + webpack: {}, + sources: { + RawSource: jest.fn().mockImplementation((json) => ({ + source: () => json, + })), + }, +})) + +// Mock dependencies +jest.mock('../../../shared/lib/constants', () => ({ + APP_BUILD_MANIFEST: 'app-build-manifest.json', + CLIENT_STATIC_FILES_RUNTIME_MAIN_APP: 'main-app', + SYSTEM_ENTRYPOINTS: new Set(['main', 'main-app']), +})) + +jest.mock('./build-manifest-plugin', () => ({ + getEntrypointFiles: jest.fn(), +})) + +jest.mock('../../../server/get-app-route-from-entrypoint', () => jest.fn()) + +jest.mock('../../../lib/is-app-page-route', () => ({ + isAppPageRoute: jest.fn(), +})) + +jest.mock('../../../lib/is-app-route-route', () => ({ + isAppRouteRoute: jest.fn(), +})) + +// Import the main plugin after mocks +import { AppBuildManifestPlugin } from './app-build-manifest-plugin' + +import { getEntrypointFiles } from './build-manifest-plugin' +import getAppRouteFromEntrypoint from '../../../server/get-app-route-from-entrypoint' +import { isAppPageRoute } from '../../../lib/is-app-page-route' +import { isAppRouteRoute } from '../../../lib/is-app-route-route' + +const mockGetEntrypointFiles = getEntrypointFiles as jest.MockedFunction< + typeof getEntrypointFiles +> +const mockGetAppRouteFromEntrypoint = + getAppRouteFromEntrypoint as jest.MockedFunction< + typeof getAppRouteFromEntrypoint + > +const mockIsAppPageRoute = isAppPageRoute as jest.MockedFunction< + typeof isAppPageRoute +> +const mockIsAppRouteRoute = isAppRouteRoute as jest.MockedFunction< + typeof isAppRouteRoute +> + +// Mock webpack compilation and entrypoints based on relisten-web debug output +function createMockEntrypoint(name: string, files: string[] = []) { + return { + name, + chunks: [], + getFiles: () => files, + } +} + +function createMockCompilation() { + const entrypoints = new Map([ + // System entrypoints (should be skipped) + ['main', createMockEntrypoint('main')], + [ + 'main-app', + createMockEntrypoint('main-app', ['static/chunks/main-app-123.js']), + ], + + // Route entrypoints (should be included) + [ + 'app/(browse)/page', + createMockEntrypoint('app/(browse)/page', [ + 'static/chunks/browse-page.js', + ]), + ], + [ + 'app/api/status/route', + createMockEntrypoint('app/api/status/route', [ + 'static/chunks/api-status.js', + ]), + ], + [ + 'app/album-art/route', + createMockEntrypoint('app/album-art/route', [ + 'static/chunks/album-art.js', + ]), + ], + + // Segment entrypoints (should NOT be included as separate entries) + [ + 'app/layout', + createMockEntrypoint('app/layout', ['static/chunks/layout.js']), + ], + [ + 'app/global-error', + createMockEntrypoint('app/global-error', [ + 'static/chunks/global-error.js', + ]), + ], + [ + 'app/(browse)/layout', + createMockEntrypoint('app/(browse)/layout', [ + 'static/chunks/browse-layout.js', + ]), + ], + [ + 'app/(browse)/@artists/default', + createMockEntrypoint('app/(browse)/@artists/default', [ + 'static/chunks/artists-default.js', + ]), + ], + + // System built-in entrypoints (should be skipped) + [ + 'next/dist/client/components/builtin/forbidden', + createMockEntrypoint('next/dist/client/components/builtin/forbidden'), + ], + ]) + + return { + entrypoints, + emitAsset: jest.fn(), + } +} + +describe('AppBuildManifestPlugin', () => { + let plugin: AppBuildManifestPlugin + let compilation: any + + beforeEach(() => { + plugin = new AppBuildManifestPlugin() + compilation = createMockCompilation() + + // Reset all mocks + jest.clearAllMocks() + + // Setup mock implementations + mockGetEntrypointFiles.mockImplementation((entrypoint) => { + return entrypoint?.getFiles() || [] + }) + + mockGetAppRouteFromEntrypoint.mockImplementation((entryName) => { + if (entryName.startsWith('app/')) { + const route = entryName + .replace(/^app\//, '') + .replace(/(page|route)$/, '') + .replace(/\/$/, '') + return route === '' ? '/' : `/${route}` + } + return null + }) + + mockIsAppPageRoute.mockImplementation((route) => { + return route.endsWith('/page') + }) + + mockIsAppRouteRoute.mockImplementation((route) => { + return route.endsWith('/route') + }) + }) + + describe('createAsset', () => { + it('should skip system entrypoints', () => { + // @ts-ignore - accessing private method for testing + plugin.createAsset(compilation) + + // Verify system entrypoints are skipped + expect(mockGetAppRouteFromEntrypoint).not.toHaveBeenCalledWith('main') + expect(mockGetAppRouteFromEntrypoint).not.toHaveBeenCalledWith('main-app') + }) + + it('should skip builtin Next.js components', () => { + // @ts-ignore - accessing private method for testing + plugin.createAsset(compilation) + + const [, assetSource] = compilation.emitAsset.mock.calls[0] + const manifest = JSON.parse(assetSource.source()) + + // Verify builtin components don't appear in final manifest + expect(Object.keys(manifest.pages)).not.toContain('/builtin/forbidden') + }) + + it('should only include actual routes in manifest', () => { + // Setup route detection mocks + mockGetAppRouteFromEntrypoint.mockImplementation((entryName) => { + switch (entryName) { + case 'app/(browse)/page': + return '/(browse)/page' + case 'app/api/status/route': + return '/api/status/route' + case 'app/album-art/route': + return '/album-art/route' + case 'app/layout': + return '/layout' + case 'app/global-error': + return '/global-error' + case 'app/(browse)/layout': + return '/(browse)/layout' + case 'app/(browse)/@artists/default': + return '/(browse)/@artists/default' + default: + return null + } + }) + + mockIsAppPageRoute.mockImplementation((route) => { + return route === '/(browse)/page' + }) + + mockIsAppRouteRoute.mockImplementation((route) => { + return route === '/api/status/route' || route === '/album-art/route' + }) + + // @ts-ignore - accessing private method for testing + plugin.createAsset(compilation) + + const [assetName, assetSource] = compilation.emitAsset.mock.calls[0] + expect(assetName).toBe('app-build-manifest.json') + + const manifest = JSON.parse(assetSource.source()) + + // Should only contain actual routes, not segments (sorted alphabetically) + expect(Object.keys(manifest.pages).sort()).toEqual( + ['/album-art/route', '/api/status/route', '/(browse)/page'].sort() + ) + }) + + it('should include main app files and route-specific files', () => { + // Setup the compilation with a route that will be detected + const testCompilation = { + entrypoints: new Map([ + [ + 'main-app', + createMockEntrypoint('main-app', ['static/chunks/main-app-123.js']), + ], + [ + 'app/(browse)/page', + createMockEntrypoint('app/(browse)/page', [ + 'static/chunks/browse-page.js', + ]), + ], + ]), + emitAsset: jest.fn(), + } + + // Setup mocks for this test + mockGetAppRouteFromEntrypoint.mockImplementation((entryName) => { + if (entryName === 'app/(browse)/page') return '/(browse)/page' + return null + }) + mockIsAppPageRoute.mockImplementation( + (route) => route === '/(browse)/page' + ) + mockIsAppRouteRoute.mockReturnValue(false) + + // @ts-ignore - accessing private method for testing + plugin.createAsset(testCompilation) + + const [, assetSource] = testCompilation.emitAsset.mock.calls[0] + const manifest = JSON.parse(assetSource.source()) + + // Check that routes include their main app files and their own files + const browsePageFiles = manifest.pages['/(browse)/page'] + + // Should include main app files + expect(browsePageFiles).toContain('static/chunks/main-app-123.js') + // Should include the page's own files + expect(browsePageFiles).toContain('static/chunks/browse-page.js') + // Should have at least these core files + expect(browsePageFiles.length).toBeGreaterThanOrEqual(2) + }) + + it('should sort manifest keys alphabetically', () => { + // Setup multiple routes in non-alphabetical order + const unsortedCompilation = { + entrypoints: new Map([ + ['main-app', createMockEntrypoint('main-app', ['main-app.js'])], + [ + 'app/zebra/page', + createMockEntrypoint('app/zebra/page', ['zebra.js']), + ], + [ + 'app/alpha/page', + createMockEntrypoint('app/alpha/page', ['alpha.js']), + ], + [ + 'app/beta/route', + createMockEntrypoint('app/beta/route', ['beta.js']), + ], + ]), + emitAsset: jest.fn(), + } + + mockGetAppRouteFromEntrypoint.mockImplementation((entryName) => { + switch (entryName) { + case 'app/zebra/page': + return '/zebra/page' + case 'app/alpha/page': + return '/alpha/page' + case 'app/beta/route': + return '/beta/route' + default: + return null + } + }) + + mockIsAppPageRoute.mockImplementation((route) => route.endsWith('/page')) + mockIsAppRouteRoute.mockImplementation((route) => + route.endsWith('/route') + ) + + // @ts-ignore - accessing private method for testing + plugin.createAsset(unsortedCompilation) + + const [, assetSource] = unsortedCompilation.emitAsset.mock.calls[0] + const manifest = JSON.parse(assetSource.source()) + + // Keys should be sorted alphabetically + expect(Object.keys(manifest.pages)).toEqual([ + '/alpha/page', + '/beta/route', + '/zebra/page', + ]) + }) + }) + + describe('isSegmentContributingToRoute', () => { + it('should identify layout files as contributing', () => { + // @ts-ignore - accessing private method for testing + const result = plugin.isSegmentContributingToRoute( + 'app/(browse)/layout', + ['(browse)'] + ) + expect(result).toBe(true) + }) + + it('should identify error files as contributing', () => { + // @ts-ignore - accessing private method for testing + const result = plugin.isSegmentContributingToRoute('app/(browse)/error', [ + '(browse)', + ]) + expect(result).toBe(true) + }) + + it('should identify parallel route defaults as contributing', () => { + // @ts-ignore - accessing private method for testing + const result = plugin.isSegmentContributingToRoute( + 'app/(browse)/@artists/default', + ['(browse)'] + ) + expect(result).toBe(true) + }) + + it('should not identify non-special files as contributing', () => { + // @ts-ignore - accessing private method for testing + const result = plugin.isSegmentContributingToRoute( + 'app/(browse)/some-file', + ['(browse)'] + ) + expect(result).toBe(false) + }) + + it('should not identify segments from different routes as contributing', () => { + // @ts-ignore - accessing private method for testing + const result = plugin.isSegmentContributingToRoute( + 'app/(content)/layout', + ['(browse)'] + ) + expect(result).toBe(false) + }) + + it('should identify root layout as contributing to all routes', () => { + // @ts-ignore - accessing private method for testing + const result = plugin.isSegmentContributingToRoute('app/layout', [ + '(browse)', + 'artist', + ]) + expect(result).toBe(true) + }) + }) +}) diff --git a/packages/next/src/build/webpack/plugins/app-build-manifest-plugin.ts b/packages/next/src/build/webpack/plugins/app-build-manifest-plugin.ts index 3048c1469d7bc..37dce0497c066 100644 --- a/packages/next/src/build/webpack/plugins/app-build-manifest-plugin.ts +++ b/packages/next/src/build/webpack/plugins/app-build-manifest-plugin.ts @@ -6,6 +6,8 @@ import { } from '../../../shared/lib/constants' import { getEntrypointFiles } from './build-manifest-plugin' import getAppRouteFromEntrypoint from '../../../server/get-app-route-from-entrypoint' +import { isAppPageRoute } from '../../../lib/is-app-page-route' +import { isAppRouteRoute } from '../../../lib/is-app-route-route' export type AppBuildManifest = { pages: Record @@ -37,12 +39,12 @@ export class AppBuildManifestPlugin { ) ) - for (const entrypoint of compilation.entrypoints.values()) { - if (!entrypoint.name) { - continue - } + // First pass: collect all entrypoints and group them by route + const allEntrypoints = new Map() + const routeToEntrypoints = new Map>() - if (SYSTEM_ENTRYPOINTS.has(entrypoint.name)) { + for (const entrypoint of compilation.entrypoints.values()) { + if (!entrypoint.name || SYSTEM_ENTRYPOINTS.has(entrypoint.name)) { continue } @@ -51,10 +53,54 @@ export class AppBuildManifestPlugin { continue } - const filesForPage = getEntrypointFiles(entrypoint) - manifest.pages[pagePath] = [...new Set([...mainFiles, ...filesForPage])] + allEntrypoints.set(entrypoint.name, entrypoint) + + // Check if this is an actual route (page or route handler) + const isRoute = isAppPageRoute(pagePath) || isAppRouteRoute(pagePath) + + if (isRoute) { + // This is a route - it gets its own manifest entry + if (!routeToEntrypoints.has(pagePath)) { + routeToEntrypoints.set(pagePath, new Set()) + } + routeToEntrypoints.get(pagePath)!.add(entrypoint.name) + + // Find all segments that contribute to this route + this.collectContributingSegments( + pagePath, + entrypoint.name, + allEntrypoints, + routeToEntrypoints + ) + } } + // Second pass: generate manifest entries for each route + for (const [routePath, entrypointNames] of routeToEntrypoints.entries()) { + const allFiles = new Set([...mainFiles]) + + for (const entrypointName of entrypointNames) { + const entrypoint = allEntrypoints.get(entrypointName) + if (entrypoint) { + const filesForEntry = getEntrypointFiles(entrypoint) + for (const file of filesForEntry) { + allFiles.add(file) + } + } + } + + manifest.pages[routePath] = Array.from(allFiles) + } + + // Sort the keys to match turbopack behavior + const sortedPages: Record = {} + Object.keys(manifest.pages) + .sort() + .forEach((key) => { + sortedPages[key] = manifest.pages[key] + }) + manifest.pages = sortedPages + const json = JSON.stringify(manifest, null, 2) compilation.emitAsset( @@ -62,4 +108,91 @@ export class AppBuildManifestPlugin { new sources.RawSource(json) as unknown as webpack.sources.RawSource ) } + + private collectContributingSegments( + routePath: string, + routeEntrypoint: string, + allEntrypoints: Map, + routeToEntrypoints: Map> + ) { + // Extract the route directory path (remove the /page or /route suffix) + const routePrefix = routeEntrypoint.replace(/\/(page|route)$/, '') + const routeParts = routePrefix + .replace(/^app\//, '') + .split('/') + .filter(Boolean) + + // Look for all segments that could contribute to this route + for (const [entrypointName] of allEntrypoints.entries()) { + if (entrypointName === routeEntrypoint) continue + + // Skip if it's already another route + const otherPagePath = getAppRouteFromEntrypoint(entrypointName) + if ( + otherPagePath && + (isAppPageRoute(otherPagePath) || isAppRouteRoute(otherPagePath)) + ) { + continue + } + + if (this.isSegmentContributingToRoute(entrypointName, routeParts)) { + routeToEntrypoints.get(routePath)!.add(entrypointName) + } + } + } + + private isSegmentContributingToRoute( + segmentEntrypoint: string, + routeParts: string[] + ): boolean { + // Remove 'app/' prefix and split into parts + const segmentPath = segmentEntrypoint.replace(/^app\//, '') + const segmentParts = segmentPath.split('/').filter(Boolean) + + if (segmentParts.length === 0) return false + + const lastSegment = segmentParts[segmentParts.length - 1] + + // Check if this is a special file that contributes to routes + const specialFiles = [ + 'layout', + 'loading', + 'error', + 'not-found', + 'global-error', + 'template', + 'default', + ] + const isSpecialFile = specialFiles.includes(lastSegment) + + // Check if this is a parallel route slot default (@slot/default) + const isParallelRouteDefault = + segmentParts.some((part) => part.startsWith('@')) && + lastSegment === 'default' + + if (!isSpecialFile && !isParallelRouteDefault) { + return false + } + + // Get the directory path of the segment (without the file name) + const segmentDir = segmentParts.slice(0, -1) + + // For parallel routes, we need special handling + if (segmentParts.some((part) => part.startsWith('@'))) { + // This is within a parallel route - it contributes if it's in the route path or a parent + const normalizedSegmentDir = segmentDir.filter( + (part) => !part.startsWith('@') + ) + return ( + normalizedSegmentDir.length <= routeParts.length && + normalizedSegmentDir.every((part, i) => routeParts[i] === part) + ) + } + + // Regular special files contribute if they are at the same level or parent level of the route + return ( + segmentDir.length <= routeParts.length && + segmentDir.every((part, i) => routeParts[i] === part) + ) + } }