From 59df4b6ec3da8113cdb38db3f75a4699378b3767 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Mon, 3 Oct 2022 15:53:28 -0700 Subject: [PATCH] Ensure entry tracing applies for app correctly (#41140) This ensures we properly detect and trace `app` dir entries and adds a regression test to ensure this is working as expected. ## Bug - [x] Related issues linked using `fixes #number` - [x] Integration tests added - [ ] Errors have a helpful link attached, see `contributing.md` --- packages/next/build/webpack-config.ts | 2 +- .../plugins/next-trace-entrypoints-plugin.ts | 47 +++++++++++++++---- .../app/dashboard/deployments/[id]/data.json | 3 ++ .../app/dashboard/deployments/[id]/page.js | 9 ++++ test/e2e/app-dir/index.test.ts | 13 +++++ 5 files changed, 64 insertions(+), 10 deletions(-) create mode 100644 test/e2e/app-dir/app/app/dashboard/deployments/[id]/data.json diff --git a/packages/next/build/webpack-config.ts b/packages/next/build/webpack-config.ts index a875d6063824..5ece05632f52 100644 --- a/packages/next/build/webpack-config.ts +++ b/packages/next/build/webpack-config.ts @@ -1817,8 +1817,8 @@ export default async function getBaseWebpackConfig( { appDir: dir, esmExternals: config.experimental.esmExternals, - staticImageImports: !config.images.disableStaticImages, outputFileTracingRoot: config.experimental.outputFileTracingRoot, + appDirEnabled: !!config.experimental.appDir, } ), // Moment.js is an extremely popular library that bundles large locale files diff --git a/packages/next/build/webpack/plugins/next-trace-entrypoints-plugin.ts b/packages/next/build/webpack/plugins/next-trace-entrypoints-plugin.ts index 007394911b99..77c1f328ac5f 100644 --- a/packages/next/build/webpack/plugins/next-trace-entrypoints-plugin.ts +++ b/packages/next/build/webpack/plugins/next-trace-entrypoints-plugin.ts @@ -24,7 +24,7 @@ const TRACE_IGNORES = [ function getModuleFromDependency( compilation: any, dep: any -): webpack.Module & { resource?: string } { +): webpack.Module & { resource?: string; request?: string } { return compilation.moduleGraph.getModule(dep) } @@ -84,30 +84,30 @@ function getFilesMapFromReasons( export class TraceEntryPointsPlugin implements webpack.WebpackPluginInstance { private appDir: string + private appDirEnabled?: boolean private tracingRoot: string private entryTraces: Map> private excludeFiles: string[] private esmExternals?: NextConfigComplete['experimental']['esmExternals'] - private staticImageImports?: boolean constructor({ appDir, + appDirEnabled, excludeFiles, esmExternals, - staticImageImports, outputFileTracingRoot, }: { appDir: string + appDirEnabled?: boolean excludeFiles?: string[] - staticImageImports: boolean outputFileTracingRoot?: string esmExternals?: NextConfigComplete['experimental']['esmExternals'] }) { this.appDir = appDir this.entryTraces = new Map() this.esmExternals = esmExternals + this.appDirEnabled = appDirEnabled this.excludeFiles = excludeFiles || [] - this.staticImageImports = staticImageImports this.tracingRoot = outputFileTracingRoot || appDir } @@ -257,15 +257,44 @@ export class TraceEntryPointsPlugin implements webpack.WebpackPluginInstance { finishModulesSpan.traceChild('get-entries').traceFn(() => { compilation.entries.forEach((entry, name) => { - if (name?.replace(/\\/g, '/').startsWith('pages/')) { + const normalizedName = name?.replace(/\\/g, '/') + + const isPage = normalizedName.startsWith('pages/') + const isApp = + this.appDirEnabled && normalizedName.startsWith('app/') + + if (isApp || isPage) { for (const dep of entry.dependencies) { if (!dep) continue const entryMod = getModuleFromDependency(compilation, dep) + // since app entries are wrapped in next-app-loader + // we need to pull the original pagePath for + // referencing during tracing + if (isApp && entryMod.request) { + const loaderQueryIdx = entryMod.request.indexOf('?') + + const loaderQuery = new URLSearchParams( + entryMod.request.substring(loaderQueryIdx) + ) + const resource = + loaderQuery + .get('pagePath') + ?.replace( + 'private-next-app-dir', + nodePath.join(this.appDir, 'app') + ) || '' + + entryModMap.set(resource, entryMod) + entryNameMap.set(resource, name) + } + if (entryMod && entryMod.resource) { - if ( - entryMod.resource.replace(/\\/g, '/').includes('pages/') - ) { + const normalizedResource = entryMod.resource.replace( + /\\/g, + '/' + ) + if (normalizedResource.includes('pages/')) { entryNameMap.set(entryMod.resource, name) entryModMap.set(entryMod.resource, entryMod) } else { diff --git a/test/e2e/app-dir/app/app/dashboard/deployments/[id]/data.json b/test/e2e/app-dir/app/app/dashboard/deployments/[id]/data.json new file mode 100644 index 000000000000..f2a886f39de7 --- /dev/null +++ b/test/e2e/app-dir/app/app/dashboard/deployments/[id]/data.json @@ -0,0 +1,3 @@ +{ + "hello": "world" +} diff --git a/test/e2e/app-dir/app/app/dashboard/deployments/[id]/page.js b/test/e2e/app-dir/app/app/dashboard/deployments/[id]/page.js index 7d548c42261d..2dd26002d29a 100644 --- a/test/e2e/app-dir/app/app/dashboard/deployments/[id]/page.js +++ b/test/e2e/app-dir/app/app/dashboard/deployments/[id]/page.js @@ -1,6 +1,15 @@ import { experimental_use as use } from 'react' +import fs from 'fs' +import path from 'path' async function getData({ params }) { + const data = JSON.parse( + fs.readFileSync( + path.join(process.cwd(), 'app/dashboard/deployments/[id]/data.json') + ) + ) + console.log('data.json', data) + return { id: params.id, } diff --git a/test/e2e/app-dir/index.test.ts b/test/e2e/app-dir/index.test.ts index 522efeead5ef..40ffd322f0c8 100644 --- a/test/e2e/app-dir/index.test.ts +++ b/test/e2e/app-dir/index.test.ts @@ -35,6 +35,19 @@ describe('app dir', () => { }) afterAll(() => next.destroy()) + if ((global as any).isNextStart) { + it('should generate build traces correctly', async () => { + const trace = JSON.parse( + await next.readFile( + '.next/server/app/dashboard/deployments/[id]/page.js.nft.json' + ) + ) as { files: string[] } + expect(trace.files.some((file) => file.endsWith('data.json'))).toBe( + true + ) + }) + } + it('should use application/octet-stream for flight', async () => { const res = await fetchViaHTTP( next.url,