Skip to content

Commit

Permalink
Merge branch 'master' into clarify-java-requirements
Browse files Browse the repository at this point in the history
  • Loading branch information
joehan authored Oct 25, 2024
2 parents d5c5bd9 + 2248973 commit 43d9c9d
Show file tree
Hide file tree
Showing 6 changed files with 77 additions and 30 deletions.
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)
6 changes: 3 additions & 3 deletions scripts/clean-install.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
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 @@ import {
getMiddlewareMatcherRegexes,
getNonStaticRoutes,
getNonStaticServerComponents,
getHeadersFromMetaFiles,
getAppMetadataFromMetaFiles,
cleanI18n,
getNextVersion,
hasStaticAppNotFoundComponent,
Expand Down Expand Up @@ -252,14 +252,18 @@ export async function build(
]);

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 @@ 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) {
Expand All @@ -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");
Expand Down Expand Up @@ -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 (
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

0 comments on commit 43d9c9d

Please sign in to comment.