From 631da2376f579534333ce8e198d1e94a13b2b743 Mon Sep 17 00:00:00 2001 From: joehan Date: Thu, 24 Oct 2024 15:54:21 -0400 Subject: [PATCH 1/2] Pin clean-publish to 5.0.0 (#7870) --- scripts/clean-install.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scripts/clean-install.sh b/scripts/clean-install.sh index a33539ef86d..ce66373508f 100755 --- a/scripts/clean-install.sh +++ b/scripts/clean-install.sh @@ -10,9 +10,9 @@ function cleanup() { trap cleanup EXIT rm -rf ./clean || true -echo "Running clean-publish --without-publish, as we would before publishing to npm..." -npx --yes clean-publish --without-publish --before-script ./scripts/clean-shrinkwrap.sh --temp-dir clean -echo "Ran clean-publish --without-publish." +echo "Running clean-publish@5.0.0 --without-publish, as we would before publishing to npm..." +npx --yes clean-publish@5.0.0 --without-publish --before-script ./scripts/clean-shrinkwrap.sh --temp-dir clean +echo "Ran clean-publish@5.0.0 --without-publish." echo "Packaging cleaned firebase-tools..." cd ./clean PACKED=$(npm pack --pack-destination ./ | tail -n 1) From 22489732ba9e2ead888ffce7a1146c7c531d531b Mon Sep 17 00:00:00 2001 From: Chalo Salvador Date: Fri, 25 Oct 2024 09:45:29 +0200 Subject: [PATCH 2/2] Skipping partial html files generated due to enabling ppr in Nextjs (#7625) * Skipping partial html files generated due to enabling ppr in Nextjs * Detect partial html files in build and add them to reasonsForBackend. - Move common logic to utils functions * Add tests cases for new utils functions * Check if file .html exists before trying to read it. * Remove warning if file does not exist * Changelog * Promise.all test isPartialHTML * Refactor to use .meta files for PPR * Changelog --- CHANGELOG.md | 1 + src/frameworks/next/index.ts | 22 +++++++++-- src/frameworks/next/testing/app.ts | 1 + src/frameworks/next/utils.spec.ts | 63 +++++++++++++++++++++--------- src/frameworks/next/utils.ts | 14 ++++--- 5 files changed, 74 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e69de29bb2d..4e7058f86bc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -0,0 +1 @@ +- Fixed Next.js issue with PPR routes not rendering correctly. (#7625) diff --git a/src/frameworks/next/index.ts b/src/frameworks/next/index.ts index dcc88846ca3..42556f5faa6 100644 --- a/src/frameworks/next/index.ts +++ b/src/frameworks/next/index.ts @@ -49,7 +49,7 @@ import { getMiddlewareMatcherRegexes, getNonStaticRoutes, getNonStaticServerComponents, - getHeadersFromMetaFiles, + getAppMetadataFromMetaFiles, cleanI18n, getNextVersion, hasStaticAppNotFoundComponent, @@ -252,7 +252,7 @@ export async function build( ]); if (appPathRoutesManifest) { - const headersFromMetaFiles = await getHeadersFromMetaFiles( + const { headers: headersFromMetaFiles, pprRoutes } = await getAppMetadataFromMetaFiles( dir, distDir, baseUrl, @@ -260,6 +260,10 @@ export async function build( ); headers.push(...headersFromMetaFiles); + for (const route of pprRoutes) { + reasonsForBackend.add(`route with ppr ${route}`); + } + if (appPathsManifest) { const unrenderedServerComponents = getNonStaticServerComponents( appPathsManifest, @@ -487,6 +491,13 @@ export async function ɵcodegenPublicDirectory( ...pagesManifestLikePrerender, }; + const { pprRoutes } = await getAppMetadataFromMetaFiles( + sourceDir, + distDir, + basePath, + appPathRoutesManifest, + ); + await Promise.all( Object.entries(routesToCopy).map(async ([path, route]) => { if (route.initialRevalidateSeconds) { @@ -504,7 +515,6 @@ export async function ɵcodegenPublicDirectory( logger.debug(`skipping ${path} due to server action`); return; } - const appPathRoute = route.srcRoute && appPathRoutesEntries.find(([, it]) => it === route.srcRoute)?.[0]; const contentDist = join(sourceDir, distDir, "server", appPathRoute ? "app" : "pages"); @@ -536,6 +546,12 @@ export async function ɵcodegenPublicDirectory( let defaultDestPath = isDefaultLocale && join(destDir, basePath, ...destPartsOrIndex); if (!fileExistsSync(sourcePath) && fileExistsSync(`${sourcePath}.html`)) { sourcePath += ".html"; + + if (pprRoutes.includes(path)) { + logger.debug(`skipping ${path} due to ppr`); + return; + } + if (localizedDestPath) localizedDestPath += ".html"; if (defaultDestPath) defaultDestPath += ".html"; } else if ( diff --git a/src/frameworks/next/testing/app.ts b/src/frameworks/next/testing/app.ts index e4e77b63817..e29d402db88 100644 --- a/src/frameworks/next/testing/app.ts +++ b/src/frameworks/next/testing/app.ts @@ -16,6 +16,7 @@ export const appPathRoutesManifest: AppPathRoutesManifest = { "/server-action/page": "/server-action", "/ssr/page": "/ssr", "/server-action/edge/page": "/server-action/edge", + "/ppr/page": "/ppr", }; export const pagesManifest: PagesManifest = { diff --git a/src/frameworks/next/utils.spec.ts b/src/frameworks/next/utils.spec.ts index a4e1d63226a..65eea3f100e 100644 --- a/src/frameworks/next/utils.spec.ts +++ b/src/frameworks/next/utils.spec.ts @@ -32,7 +32,7 @@ import { getMiddlewareMatcherRegexes, getNonStaticRoutes, getNonStaticServerComponents, - getHeadersFromMetaFiles, + getAppMetadataFromMetaFiles, isUsingNextImageInAppDirectory, getNextVersion, getRoutesWithServerAction, @@ -472,38 +472,63 @@ describe("Next.js utils", () => { }); }); - describe("getHeadersFromMetaFiles", () => { + describe("getAppMetadataFromMetaFiles", () => { let sandbox: sinon.SinonSandbox; beforeEach(() => (sandbox = sinon.createSandbox())); afterEach(() => sandbox.restore()); - it("should get headers from meta files", async () => { + it("should return the correct headers and pprRoutes from meta files", async () => { const distDir = ".next"; const readJsonStub = sandbox.stub(frameworksUtils, "readJSON"); const dirExistsSyncStub = sandbox.stub(fsUtils, "dirExistsSync"); const fileExistsSyncStub = sandbox.stub(fsUtils, "fileExistsSync"); + // /api/static dirExistsSyncStub.withArgs(`${distDir}/server/app/api/static`).returns(true); fileExistsSyncStub.withArgs(`${distDir}/server/app/api/static.meta`).returns(true); readJsonStub.withArgs(`${distDir}/server/app/api/static.meta`).resolves(metaFileContents); + // /ppr + dirExistsSyncStub.withArgs(`${distDir}/server/app/ppr`).returns(true); + fileExistsSyncStub.withArgs(`${distDir}/server/app/ppr.meta`).returns(true); + readJsonStub.withArgs(`${distDir}/server/app/ppr.meta`).resolves({ + ...metaFileContents, + postponed: "true", + }); + expect( - await getHeadersFromMetaFiles(".", distDir, "/asdf", appPathRoutesManifest), - ).to.deep.equal([ - { - source: "/asdf/api/static", - headers: [ - { - key: "content-type", - value: "application/json", - }, - { - key: "custom-header", - value: "custom-value", - }, - ], - }, - ]); + await getAppMetadataFromMetaFiles(".", distDir, "/asdf", appPathRoutesManifest), + ).to.deep.equal({ + headers: [ + { + source: "/asdf/api/static", + headers: [ + { + key: "content-type", + value: "application/json", + }, + { + key: "custom-header", + value: "custom-value", + }, + ], + }, + { + source: "/asdf/ppr", + headers: [ + { + key: "content-type", + value: "application/json", + }, + { + key: "custom-header", + value: "custom-value", + }, + ], + }, + ], + pprRoutes: ["/ppr"], + }); }); }); diff --git a/src/frameworks/next/utils.ts b/src/frameworks/next/utils.ts index 3ccd3d374c7..50e02d020db 100644 --- a/src/frameworks/next/utils.ts +++ b/src/frameworks/next/utils.ts @@ -365,15 +365,16 @@ export function getNonStaticServerComponents( } /** - * Get headers from .meta files + * Get metadata from .meta files */ -export async function getHeadersFromMetaFiles( +export async function getAppMetadataFromMetaFiles( sourceDir: string, distDir: string, basePath: string, appPathRoutesManifest: AppPathRoutesManifest, -): Promise { +): Promise<{ headers: HostingHeadersWithSource[]; pprRoutes: string[] }> { const headers: HostingHeadersWithSource[] = []; + const pprRoutes: string[] = []; await Promise.all( Object.entries(appPathRoutesManifest).map(async ([key, source]) => { @@ -385,17 +386,20 @@ export async function getHeadersFromMetaFiles( const metadataPath = `${routePath}.meta`; if (dirExistsSync(routePath) && fileExistsSync(metadataPath)) { - const meta = await readJSON<{ headers?: Record }>(metadataPath); + const meta = await readJSON<{ headers?: Record; postponed?: string }>( + metadataPath, + ); if (meta.headers) headers.push({ source: posix.join(basePath, source), headers: Object.entries(meta.headers).map(([key, value]) => ({ key, value })), }); + if (meta.postponed) pprRoutes.push(source); } }), ); - return headers; + return { headers, pprRoutes }; } /**