Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 7 additions & 0 deletions .changeset/silver-taxis-kick.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@opennextjs/aws": patch
---

fix(pages-router): Add patch for trustHostHeader using res.revalidate

In pages router on Next 15 and 16 if you tried to `res.revalidate` you would run into this error: `Error: Failed to revalidate /path: Invariant: missing internal router-server-methods this is an internal bug`. This PR introduces a patch that always sets `context.trustHostHeader` as true in the runtime code.
2 changes: 1 addition & 1 deletion examples/pages-router/src/middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,5 @@ export function middleware(request: NextRequest) {
}

export const config = {
matcher: ["/"],
matcher: ["/", "/revalidate/:path*"],
};
26 changes: 26 additions & 0 deletions examples/pages-router/src/pages/api/revalidate.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import type { NextApiRequest, NextApiResponse } from "next";

type Data =
| { revalidated: true; path: string }
| { revalidated: false; message: string };

export default async function handler(
req: NextApiRequest,
res: NextApiResponse<Data>,
) {
if (req.method !== "POST" && req.method !== "GET") {
return res
.status(405)
.json({ revalidated: false, message: "Method not allowed" });
}

const key = req.query.key;
if (!key || typeof key !== "string") {
return res.status(400).json({ revalidated: false, message: "Missing key" });
}

const path = `/revalidate/${key}`;

await res.revalidate(path);
return res.status(200).json({ revalidated: true, path });
}
78 changes: 78 additions & 0 deletions examples/pages-router/src/pages/revalidate/[key].tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
import type { GetStaticPropsContext, GetStaticPropsResult } from "next";
import Head from "next/head";

type Params = { key: string };

type FakeRecord = {
key: string;
title: string;
updatedAt: string;
};

type Props = {
record: FakeRecord;
};

const fakeDb: Omit<FakeRecord, "updatedAt">[] = [
{
key: "1",
title: "First record",
},
{
key: "2",
title: "Second record",
},
{
key: "3",
title: "Third record",
},
];

export default function TestKeyPage({ record }: Props) {
Comment thread
sommeeeer marked this conversation as resolved.
return (
<div style={{ padding: 24 }}>
<Head>
<title>SSG Test — {record.key}</title>
</Head>
<h1>SSG Test Page</h1>
<p>
<strong>Key:</strong> {record.key}
</p>
<p>
<strong>Title:</strong> {record.title}
</p>
<p>
<strong>Updated:</strong>{" "}
<span data-testid="updated-at">{record.updatedAt}</span>
</p>
<p>Revalidate set in getStaticProps.</p>
</div>
);
}

export async function getStaticProps({
params,
}: GetStaticPropsContext<Params>): Promise<GetStaticPropsResult<Props>> {
const found = fakeDb.find((item) => item.key === params?.key);

if (!found) {
return { notFound: true };
}

return {
props: {
record: {
...found,
updatedAt: new Date().toISOString(),
},
},
};
}

export async function getStaticPaths() {
const paths = fakeDb.map((item) => ({
params: { key: item.key },
}));

return { paths, fallback: "blocking" };
Comment thread
sommeeeer marked this conversation as resolved.
}
1 change: 1 addition & 0 deletions packages/open-next/src/build/createServerBundle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,7 @@ async function generateBundle(
patches.patchBackgroundRevalidation,
patches.patchUseCacheForISR,
patches.patchNodeEnvironment,
patches.patchPagesApiRuntimeProd,
...additionalCodePatches,
]);

Expand Down
1 change: 1 addition & 0 deletions packages/open-next/src/build/patch/patches/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,4 @@ export { patchFetchCacheSetMissingWaitUntil } from "./patchFetchCacheWaitUntil.j
export { patchBackgroundRevalidation } from "./patchBackgroundRevalidation.js";
export { patchNodeEnvironment } from "./patchNodeEnvironment.js";
export { patchOriginalNextConfig } from "./patchOriginalNextConfig.js";
export { patchPagesApiRuntimeProd } from "./patchPagesApiRuntimeProd.js";
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
import { getCrossPlatformPathRegex } from "utils/regex.js";
import { createPatchCode } from "../astCodePatcher.js";
import type { CodePatcher } from "../codePatcher.js";

// `context.trustHostHeader` is undefined in our case
// Trust the host header when invoking `res.revalidate("/path")` from pages router
// https://github.com/vercel/next.js/blob/178a4c7/packages/next/src/server/api-utils/node/api-resolver.ts#L301
export const trustHostHeaderRule = `
rule:
kind: member_expression
pattern: $CONTEXT.trustHostHeader
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like there are 3 occurences of context.tustHostHeader in this function, what is the rationale for only updating one?

inside:
kind: parenthesized_expression
inside:
kind: if_statement
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should look at making this simpler.
If not possible, it would be nice to add what code we are looking for in the JSDoc

all:
- has:
regex: await
kind: statement_block
has:
kind: lexical_declaration
regex: HEAD
has:
kind: variable_declarator
has:
kind: await_expression
has:
kind: call_expression
has:
kind: identifier
regex: ^fetch$
fix:
'true'
`;

// Use correct protocol when doing HEAD fetch for revalidation
export const headFetchProtocolRule = `
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment here, could you please add what snippet we are tryin to match. We might be able to come with a more readable rule too.

rule:
kind: string_fragment
regex: ^https://
inside:
kind: template_string
inside:
kind: arguments
has:
kind: object
regex: HEAD
inside:
kind: call_expression
inside:
kind: await_expression
regex: fetch
inside:
kind: variable_declarator
inside:
kind: lexical_declaration
regex: x-vercel-cache
inside:
kind: statement_block
inside:
kind: if_statement
fix:
'\${r.headers["x-forwarded-proto"] || "https"}://'
`;

const pathFilter = getCrossPlatformPathRegex(
String.raw`/next/dist/compiled/next-server/pages-api(-turbo)?\.runtime\.prod\.js$`,
{
escape: false,
},
);

export const patchPagesApiRuntimeProd: CodePatcher = {
name: "patch-pages-api-runtime-prod",
patches: [
// Trust the host header when invoking `res.revalidate("") from pages router
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's ok to only comment on trustHostHeaderRule ?

Suggested change
// Trust the host header when invoking `res.revalidate("") from pages router

{
pathFilter,
contentFilter: /trustHostHeader/,
patchCode: createPatchCode(trustHostHeaderRule),
versions: ">=15.0.0",
Comment thread
sommeeeer marked this conversation as resolved.
},
// Use correct protocol when doing HEAD fetch for revalidation
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

Suggested change
// Use correct protocol when doing HEAD fetch for revalidation

{
pathFilter,
contentFilter: /https/,
patchCode: createPatchCode(headFetchProtocolRule),
versions: ">=15.0.0",
Comment thread
sommeeeer marked this conversation as resolved.
},
],
};
32 changes: 32 additions & 0 deletions packages/tests-e2e/tests/pagesRouter/revalidate.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { expect, test } from "@playwright/test";

test("res.revalidate should work", async ({ page }) => {
// Load the page initially and get the initial updatedAt value
await page.goto("/revalidate/1");
const initialUpdatedAt = await page.getByTestId("updated-at").textContent();
Comment thread
sommeeeer marked this conversation as resolved.
expect(initialUpdatedAt).toBe(initialUpdatedAt);

// Reload the page again to ensure its SSG
await page.reload();
const reloadedUpdatedAt = await page.getByTestId("updated-at").textContent();
expect(reloadedUpdatedAt).toBe(initialUpdatedAt);

// Trigger revalidation via the API route
const res = await page.request.post("/api/revalidate/?key=1");
const json = await res.json();

expect(json).toEqual({ revalidated: true, path: "/revalidate/1" });

if (!res.ok()) {
throw new Error(`Failed to trigger revalidation: ${await res.text()}`);
}

// Reload the page to get the updated content after revalidation
// It should be greater than `initialUpdatedAt`
await page.reload();

const updatedUpdatedAt = await page.getByTestId("updated-at").textContent();
expect(new Date(updatedUpdatedAt!).getTime()).toBeGreaterThan(
new Date(initialUpdatedAt!).getTime(),
);
});
Loading
Loading