-
Notifications
You must be signed in to change notification settings - Fork 204
fix(pages-router): Add patch for trustHostHeader using res.revalidate #1109
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
0c17c5e
2e49c07
3c09aac
c852f60
40d0aed
4097b2c
7df956b
3492764
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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. |
| 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 }); | ||
| } |
| 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) { | ||
| 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" }; | ||
|
sommeeeer marked this conversation as resolved.
|
||
| } | ||
| 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 | ||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like there are 3 occurences of |
||||
| inside: | ||||
| kind: parenthesized_expression | ||||
| inside: | ||||
| kind: if_statement | ||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should look at making this simpler. |
||||
| 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 = ` | ||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's ok to only comment on
Suggested change
|
||||
| { | ||||
| pathFilter, | ||||
| contentFilter: /trustHostHeader/, | ||||
| patchCode: createPatchCode(trustHostHeaderRule), | ||||
| versions: ">=15.0.0", | ||||
|
sommeeeer marked this conversation as resolved.
|
||||
| }, | ||||
| // Use correct protocol when doing HEAD fetch for revalidation | ||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ditto
Suggested change
|
||||
| { | ||||
| pathFilter, | ||||
| contentFilter: /https/, | ||||
| patchCode: createPatchCode(headFetchProtocolRule), | ||||
| versions: ">=15.0.0", | ||||
|
sommeeeer marked this conversation as resolved.
|
||||
| }, | ||||
| ], | ||||
| }; | ||||
| 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(); | ||
|
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(), | ||
| ); | ||
| }); | ||
Uh oh!
There was an error while loading. Please reload this page.