From 45cc801bd9150ad8d7ecc240070417e07e81bb5f Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Thu, 12 Sep 2024 11:15:10 -0700 Subject: [PATCH 1/2] Fix not-found case with incremental tracing --- .../flying-shuttle/detect-changed-entries.ts | 11 ++- .../src/build/flying-shuttle/stitch-builds.ts | 5 +- test/e2e/app-dir/app/app/not-found.js | 9 +++ test/e2e/app-dir/app/app/not-found.module.css | 3 + test/e2e/app-dir/app/flying-shuttle.test.ts | 75 ++++++++++++++++--- 5 files changed, 89 insertions(+), 14 deletions(-) create mode 100644 test/e2e/app-dir/app/app/not-found.js create mode 100644 test/e2e/app-dir/app/app/not-found.module.css diff --git a/packages/next/src/build/flying-shuttle/detect-changed-entries.ts b/packages/next/src/build/flying-shuttle/detect-changed-entries.ts index b8411119d6bbf..ace96d83fda05 100644 --- a/packages/next/src/build/flying-shuttle/detect-changed-entries.ts +++ b/packages/next/src/build/flying-shuttle/detect-changed-entries.ts @@ -241,7 +241,10 @@ export async function detectChangedEntries({ console.error('missing trace data', traceFile, normalizedEntry) } } catch (err) { - console.error(`Failed to detect change for ${entry}`, err) + console.error( + `Failed to detect change for ${entry} ${normalizedEntry}`, + err + ) } } @@ -275,7 +278,11 @@ export async function detectChangedEntries({ } for (const entry of appPaths || []) { - const normalizedEntry = getPageFromPath(entry, pageExtensions) + let normalizedEntry = getPageFromPath(entry, pageExtensions) + + if (normalizedEntry === '/not-found') { + normalizedEntry = '/_not-found/page' + } await detectChange({ entry, normalizedEntry, type: 'app' }) } diff --git a/packages/next/src/build/flying-shuttle/stitch-builds.ts b/packages/next/src/build/flying-shuttle/stitch-builds.ts index 9e4d254c9470e..274b367b7acf4 100644 --- a/packages/next/src/build/flying-shuttle/stitch-builds.ts +++ b/packages/next/src/build/flying-shuttle/stitch-builds.ts @@ -141,6 +141,9 @@ export async function stitchBuilds( if (normalizedEntry === '/') { normalizedEntry = '/index' } + if (normalizedEntry === '/not-found') { + normalizedEntry = '/_not-found/page' + } await copyPageChunk(normalizedEntry, type) } finally { copySema.release() @@ -148,8 +151,6 @@ export async function stitchBuilds( }) ) } - // always attempt copying not-found chunk - await copyPageChunk('/_not-found/page', 'app').catch(() => {}) // merge dynamic/static routes in routes-manifest const [restoreRoutesManifest, currentRoutesManifest] = await Promise.all( diff --git a/test/e2e/app-dir/app/app/not-found.js b/test/e2e/app-dir/app/app/not-found.js new file mode 100644 index 0000000000000..95664cc1214bb --- /dev/null +++ b/test/e2e/app-dir/app/app/not-found.js @@ -0,0 +1,9 @@ +import * as styles from './not-found.module.css' + +export default function Page() { + return ( + <> +

top-level not found page

+ + ) +} diff --git a/test/e2e/app-dir/app/app/not-found.module.css b/test/e2e/app-dir/app/app/not-found.module.css new file mode 100644 index 0000000000000..6335e087d5ec7 --- /dev/null +++ b/test/e2e/app-dir/app/app/not-found.module.css @@ -0,0 +1,3 @@ +.text { + color: cyan; +} diff --git a/test/e2e/app-dir/app/flying-shuttle.test.ts b/test/e2e/app-dir/app/flying-shuttle.test.ts index 60ab467f2bfd0..5212603a3dcd8 100644 --- a/test/e2e/app-dir/app/flying-shuttle.test.ts +++ b/test/e2e/app-dir/app/flying-shuttle.test.ts @@ -37,6 +37,11 @@ import { nextTestSetup, isNextStart } from 'e2e-utils' initialConfig = manifest.config }) + function checkErrorLogs() { + expect(next.cliOutput).not.toContain('ENOENT') + expect(next.cliOutput).not.toContain('Failed to detect change') + } + async function checkShuttleManifest() { const manifest = await next.readJSON( '.next/cache/shuttle/shuttle-manifest.json' @@ -48,6 +53,14 @@ import { nextTestSetup, isNextStart } from 'e2e-utils' }) } + async function nextStart() { + // our initial build was built in store-only mode so + // enable full version in successive builds + delete next.env['NEXT_PRIVATE_FLYING_SHUTTLE_STORE_ONLY'] + next.env['NEXT_PRIVATE_FLYING_SHUTTLE'] = '1' + await next.start() + } + it('should have file hashes in trace files', async () => { const deploymentsTracePath = '.next/server/app/dashboard/deployments/[id]/page.js.nft.json' @@ -199,14 +212,20 @@ import { nextTestSetup, isNextStart } from 'e2e-utils' content: 'hello from app/dashboard/deployments/[id]', type: 'app', }, + { + path: '/non-existent/path', + content: 'top-level not found page', + type: 'app', + status: 404, + }, ] for (const testPath of testPaths) { - const { path, content } = testPath + const { path, content, status } = testPath require('console').error('checking', path) const res = await next.fetch(path) - expect(res.status).toBe(200) + expect(res.status).toBe(status || 200) const browser = await next.browser(path) @@ -252,11 +271,6 @@ import { nextTestSetup, isNextStart } from 'e2e-utils' } it('should only rebuild just a changed app route correctly', async () => { - // our initial build was built in store-only mode so - // enable full version in successive builds - delete next.env['NEXT_PRIVATE_FLYING_SHUTTLE_STORE_ONLY'] - next.env['NEXT_PRIVATE_FLYING_SHUTTLE'] = '1' - await next.stop() const dataPath = 'app/dashboard/deployments/[id]/data.json' @@ -264,8 +278,10 @@ import { nextTestSetup, isNextStart } from 'e2e-utils' try { await next.patchFile(dataPath, JSON.stringify({ hello: 'again' })) - await next.start() + await nextStart() + checkErrorLogs() + expect(next.cliOutput).not.toContain('/not-found') expect(next.cliOutput).not.toContain('/catch-all') expect(next.cliOutput).not.toContain('/blog/[slug]') expect(next.cliOutput).toContain('/dashboard/deployments/[id]') @@ -291,9 +307,12 @@ import { nextTestSetup, isNextStart } from 'e2e-utils' 'hello from pages/index!!' ) ) - await next.start() + await nextStart() + + checkErrorLogs() expect(next.cliOutput).toContain('/') + expect(next.cliOutput).not.toContain('/not-found') expect(next.cliOutput).not.toContain('/catch-all') expect(next.cliOutput).not.toContain('/blog/[slug]') @@ -321,10 +340,13 @@ import { nextTestSetup, isNextStart } from 'e2e-utils' ) ) await next.patchFile(dataPath, JSON.stringify({ hello: 'again' })) - await next.start() + await nextStart() + + checkErrorLogs() expect(next.cliOutput).toContain('/') expect(next.cliOutput).toContain('/dashboard/deployments/[id]') + expect(next.cliOutput).not.toContain('/not-found') expect(next.cliOutput).not.toContain('/catch-all') expect(next.cliOutput).not.toContain('/blog/[slug]') @@ -335,5 +357,38 @@ import { nextTestSetup, isNextStart } from 'e2e-utils' await next.patchFile(dataPath, originalDataContent) } }) + + it('should rebuild not-found when it changed', async () => { + await next.stop() + + const dataPath = 'app/not-found.module.css' + const originalDataContent = await next.readFile(dataPath) + + try { + await next.patchFile( + dataPath, + originalDataContent.replace('cyan', 'pink') + ) + await nextStart() + + checkErrorLogs() + + expect(next.cliOutput).toContain('/not-found') + + const browser = await next.browser('/non-existent/path') + await retry(async () => { + expect( + await browser.eval( + 'getComputedStyle(document.querySelector("p")).color' + ) + ).toBe('rgb(255, 192, 203)') + }) + + await checkShuttleManifest() + await checkAppPagesNavigation() + } finally { + await next.patchFile(dataPath, originalDataContent) + } + }) } ) From 356641072b44d450e92dffe784487dac6abf4a7d Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Thu, 12 Sep 2024 11:28:09 -0700 Subject: [PATCH 2/2] update test --- test/e2e/app-dir/app/app/not-found.js | 2 +- test/e2e/app-dir/app/flying-shuttle.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/e2e/app-dir/app/app/not-found.js b/test/e2e/app-dir/app/app/not-found.js index 95664cc1214bb..e15b304d64fdb 100644 --- a/test/e2e/app-dir/app/app/not-found.js +++ b/test/e2e/app-dir/app/app/not-found.js @@ -3,7 +3,7 @@ import * as styles from './not-found.module.css' export default function Page() { return ( <> -

top-level not found page

+

This page could not be found

) } diff --git a/test/e2e/app-dir/app/flying-shuttle.test.ts b/test/e2e/app-dir/app/flying-shuttle.test.ts index 5212603a3dcd8..4656caae05616 100644 --- a/test/e2e/app-dir/app/flying-shuttle.test.ts +++ b/test/e2e/app-dir/app/flying-shuttle.test.ts @@ -214,7 +214,7 @@ import { nextTestSetup, isNextStart } from 'e2e-utils' }, { path: '/non-existent/path', - content: 'top-level not found page', + content: 'This page could not be found', type: 'app', status: 404, },