Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
0a303e8
Skipping partial html files generated due to enabling ppr in Nextjs
chalosalvador Sep 4, 2024
703b6a9
Detect partial html files in build and add them to reasonsForBackend.
chalosalvador Sep 12, 2024
c4eedfe
Add tests cases for new utils functions
chalosalvador Sep 12, 2024
983cd5d
Merge branch 'master' of github.com:firebase/firebase-tools into chal…
chalosalvador Sep 12, 2024
c78c861
Merge branch 'master' into chalosalvador/fix-next-ppr-dynamic-pages
chalosalvador Sep 12, 2024
467392a
Merge branch 'master' into chalosalvador/fix-next-ppr-dynamic-pages
chalosalvador Oct 3, 2024
ec84e82
Merge branch 'chalosalvador/fix-next-ppr-dynamic-pages' of github.com…
chalosalvador Oct 4, 2024
5aab663
Check if file .html exists before trying to read it.
chalosalvador Oct 4, 2024
c86c96a
Remove warning if file does not exist
chalosalvador Oct 4, 2024
0f0ea1a
Merge branch 'master' into chalosalvador/fix-next-ppr-dynamic-pages
chalosalvador Oct 4, 2024
74ec984
Changelog
chalosalvador Oct 4, 2024
f04ed22
Promise.all test isPartialHTML
chalosalvador Oct 9, 2024
d81127e
Merge branch 'master' of github.com:firebase/firebase-tools into chal…
chalosalvador Oct 9, 2024
005d6d3
Merge branch 'master' into chalosalvador/fix-next-ppr-dynamic-pages
chalosalvador Oct 17, 2024
47c3def
Refactor to use .meta files for PPR
chalosalvador Oct 17, 2024
9dd127a
Changelog
chalosalvador Oct 17, 2024
2d1eb4a
Merge branch 'master' into chalosalvador/fix-next-ppr-dynamic-pages
chalosalvador Oct 22, 2024
f007b25
Merge branch 'master' into chalosalvador/fix-next-ppr-dynamic-pages
chalosalvador Oct 24, 2024
ff589a5
Update CHANGELOG.md
leoortizz Oct 24, 2024
17ca89c
Update CHANGELOG.md
leoortizz Oct 24, 2024
b5a67be
Merge branch 'master' of github.com:firebase/firebase-tools into chal…
chalosalvador Oct 25, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- Fixed Next.js issue with PPR routes not rendering correctly. (#7625)
22 changes: 19 additions & 3 deletions src/frameworks/next/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
getMiddlewareMatcherRegexes,
getNonStaticRoutes,
getNonStaticServerComponents,
getHeadersFromMetaFiles,
getAppMetadataFromMetaFiles,
cleanI18n,
getNextVersion,
hasStaticAppNotFoundComponent,
Expand Down Expand Up @@ -97,13 +97,13 @@
const DEFAULT_NUMBER_OF_REASONS_TO_LIST = 5;

function getReactVersion(cwd: string): string | undefined {
return findDependency("react-dom", { cwd, omitDev: false })?.version;

Check warning on line 100 in src/frameworks/next/index.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe return of an `any` typed value

Check warning on line 100 in src/frameworks/next/index.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe member access .version on an `any` value
}

/**
* Returns whether this codebase is a Next.js backend.
*/
export async function discover(dir: string) {

Check warning on line 106 in src/frameworks/next/index.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Missing return type on function
if (!(await pathExists(join(dir, "package.json")))) return;
const version = getNextVersion(dir);
if (!(await whichNextConfigFile(dir)) && !version) return;
Expand Down Expand Up @@ -160,10 +160,10 @@

const nextBuild = new Promise((resolve, reject) => {
const buildProcess = spawn(cli, ["build"], { cwd: dir, env });
buildProcess.stdout?.on("data", (data) => logger.info(data.toString()));

Check warning on line 163 in src/frameworks/next/index.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe argument of type `any` assigned to a parameter of type `Error`

Check warning on line 163 in src/frameworks/next/index.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe member access .toString on an `any` value

Check warning on line 163 in src/frameworks/next/index.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe call of an `any` typed value
buildProcess.stderr?.on("data", (data) => logger.info(data.toString()));

Check warning on line 164 in src/frameworks/next/index.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe argument of type `any` assigned to a parameter of type `Error`

Check warning on line 164 in src/frameworks/next/index.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe member access .toString on an `any` value

Check warning on line 164 in src/frameworks/next/index.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe call of an `any` typed value
buildProcess.on("error", (err) => {
reject(new FirebaseError(`Unable to build your Next.js app: ${err}`));

Check warning on line 166 in src/frameworks/next/index.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Invalid type "Error" of template literal expression
});
buildProcess.on("exit", (code) => {
resolve(code);
Expand Down Expand Up @@ -252,14 +252,18 @@
]);

if (appPathRoutesManifest) {
const headersFromMetaFiles = await getHeadersFromMetaFiles(
const { headers: headersFromMetaFiles, pprRoutes } = await getAppMetadataFromMetaFiles(
dir,
distDir,
baseUrl,
appPathRoutesManifest,
);
headers.push(...headersFromMetaFiles);

for (const route of pprRoutes) {
reasonsForBackend.add(`route with ppr ${route}`);
}

if (appPathsManifest) {
const unrenderedServerComponents = getNonStaticServerComponents(
appPathsManifest,
Expand Down Expand Up @@ -487,6 +491,13 @@
...pagesManifestLikePrerender,
};

const { pprRoutes } = await getAppMetadataFromMetaFiles(
sourceDir,
distDir,
basePath,
appPathRoutesManifest,
);

await Promise.all(
Object.entries(routesToCopy).map(async ([path, route]) => {
if (route.initialRevalidateSeconds) {
Expand All @@ -504,7 +515,6 @@
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");
Expand Down Expand Up @@ -536,6 +546,12 @@
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 (
Expand Down
1 change: 1 addition & 0 deletions src/frameworks/next/testing/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down
63 changes: 44 additions & 19 deletions src/frameworks/next/utils.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import {
getMiddlewareMatcherRegexes,
getNonStaticRoutes,
getNonStaticServerComponents,
getHeadersFromMetaFiles,
getAppMetadataFromMetaFiles,
isUsingNextImageInAppDirectory,
getNextVersion,
getRoutesWithServerAction,
Expand Down Expand Up @@ -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"],
});
});
});

Expand Down
14 changes: 9 additions & 5 deletions src/frameworks/next/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<HostingHeadersWithSource[]> {
): Promise<{ headers: HostingHeadersWithSource[]; pprRoutes: string[] }> {
const headers: HostingHeadersWithSource[] = [];
const pprRoutes: string[] = [];

await Promise.all(
Object.entries(appPathRoutesManifest).map(async ([key, source]) => {
Expand All @@ -385,17 +386,20 @@ export async function getHeadersFromMetaFiles(
const metadataPath = `${routePath}.meta`;

if (dirExistsSync(routePath) && fileExistsSync(metadataPath)) {
const meta = await readJSON<{ headers?: Record<string, string> }>(metadataPath);
const meta = await readJSON<{ headers?: Record<string, string>; 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 };
}

/**
Expand Down
Loading