fix(pages): refresh next/head tags when regenerating ISR HTML - #2729
Open
NathanDrake2406 wants to merge 4 commits into
Open
fix(pages): refresh next/head tags when regenerating ISR HTML#2729NathanDrake2406 wants to merge 4 commits into
NathanDrake2406 wants to merge 4 commits into
Conversation
ISR regeneration re-renders the page body but reuses the cached shell verbatim, so the <head> stays frozen at whatever the first cache-filling render produced. A page whose title or meta tags derive from getStaticProps data serves an updated body under permanently stale metadata, and the two never reconverge for the lifetime of the entry. next/head tags accumulate as a side effect of rendering into an AsyncLocalStorage scope that closes when the regeneration pass returns, so the refreshed head has to be read inside that pass. The render callback now accepts an onHeadReady hook that runs after the render and before the scope unwinds. The collected tags are located in the cached shell through the data-next-head attribute that getSSRHeadHTML stamps on everything it emits, which keeps the swap working on entries written before this change. _document.getInitialProps runs in the same hook because its head tags share that collector; omitting it would drop them from the regenerated shell. _document-rendered head children and CSS-in-JS styles still come from the cached shell, since regeneration does not re-render _document.
Contributor
Author
|
@codex review |
commit: |
Contributor
Performance benchmarksCompared 0 improved · 0 regressed · 6 within ±1.5%
View detailed results and traces 🟢 improvement · 🔴 regression · ⚫ change below 1.5% · paired base/head |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e5a1d2a2be
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Two problems in the regeneration head swap, both raised in review. The regeneration hook called _document.getInitialProps through the bare callDocumentGetInitialProps helper, whose context deliberately omits req, res, pathname, query and asPath and supplies a no-op renderPage. That is not the contract the initial render uses: enhancePageElement is always wired in the prod handler, so any user override resolves its head through runDocumentRenderPage with the real request context instead. A Document reading ctx.req would therefore throw (swallowed by the helper) or emit fallback tags during regeneration, and the swap would replace a complete cached head with that degraded one. Reproducing the full document pipeline on regeneration is a much larger change, so those apps now keep the cached head, which is the behaviour they had before the refresh existed. Everyone else — no _document, or one that never overrides getInitialProps — is unaffected, and for them the helper was a no-op anyway, so the hook is now just getSSRHeadHTML. Locating the closing head with indexOf also read raw-text content as markup. headChildToHTML escapes </script and </style in inline bodies but not </head>, so a next/head script may legitimately contain that string. The scan then ended mid-element, matched nothing, and silently skipped the refresh — or, with a differently shaped shell, inserted the fresh head while leaving later stale tags in place. Raw-text elements are blanked length-preservingly before the boundary lookup so indices still map onto the original shell.
… worktree-isr-head-refresh
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
next/headoutput in sync with the data an ISR regeneration rendersnext/headtags only;_document-owned shell markup stays cached<title>/meta stop serving permanently stale metadataWhy
ISR regeneration re-renders the page body but reuses the cached shell verbatim.
renderPagesIsrHtml()rewrites the body region and the__NEXT_DATA__script, and everything before<div id="__next">(the entire<head>) is carried over from the previously cached HTML.That freezes the head at whatever the first cache-filling render produced. For a page whose metadata derives from
getStaticProps:renaming the product updates the
<h1>on the next revalidation but not the<title>. Body and metadata drift apart and never reconverge for the lifetime of the entry. Next.js re-runs the whole render, including the document, on every background revalidate, so the head rotates with the data there.The reason the head was dropped rather than deliberately excluded:
next/headtags are not returned by the render, they accumulate as a side effect of it into an AsyncLocalStorage scope (runWithHeadState) that closes when the pass returns. By the timerenderPagesIsrHtml()has the fresh body string, the collected head is already out of reach. Reading it requires a hook inside the pass.What changed
next/headtags_documentoverridesgetInitialProps_document-rendered head children / CSS-in-JSstylesThe collector's tags are located in the cached shell via the
data-next-headattribute thatgetSSRHeadHTML()stamps on everything it emits, which is also what Next.js uses to reconcile the head on the client. BecausessrHeadHTMLis concatenated ahead of trace meta and_documentstyles inbuildPagesShellHtml(), the tags form one contiguous run, and replacing first-match-start through last-match-end swaps exactly that run.Why
_document.getInitialPropsapps opt outenhancePageElementis non-optional increatePagesPageHandler, so in production a user_document.getInitialPropsoverride always resolves its head throughrunDocumentRenderPage()— with a realctx.renderPageplus the request-scopedreq,res,pathname,queryandasPath.pages-page-response.tsonly falls back to the barecallDocumentGetInitialProps()on theskippedbranch, which by definition means there is no override.Regeneration cannot reproduce that context without running the whole document pipeline again, which is out of scope here. Collecting a head anyway would mean swapping a complete cached head for one built from a
_documentthat threw onctx.req(swallowed) or emitted fallback tags. So those apps keep the cached head — their pre-PR behaviour — and everyone else gets the refresh. For every app that does refresh, the_documenthelper was a no-op regardless, since it early-returns on the unmodified shim default.Locating the closing
</head>headChildToHTML()escapes</scriptand</stylein inline bodies but not</head>, so anext/headinline script may legitimately contain that string. Raw-text elements are therefore blanked (length-preservingly, so indices still map onto the original shell) before the boundary lookup, rather than reading the first</head>substring as markup.Maintainer review path
packages/vinext/src/entries/pages-server-entry.tsfor theonHeadReadyhook and why it sits inside the ALS scope.packages/vinext/src/server/pages-page-data.tsforrefreshCachedHeadTags(), the swap boundary, and the raw-text-aware</head>lookup.packages/vinext/src/server/pages-page-handler.tsforcollectIsrHeadHTMLand the_document.getInitialPropsopt-out.packages/vinext/src/server/document-initial-head.tsforhasUserDocumentGetInitialProps(), which keeps the shim-default identity check in one place.tests/pages-page-data.test.tsfor the regression proof.Validation
tests/pages-page-data.test.ts: the regenerated shell carries the freshnext/headtags while leaving_document-owned head markup in place; an empty collection does not delete the cached head; and a cached shell whose inline script body contains a literal</head>still swaps the whole run.refreshCachedHeadTags()call, and confirmed the</head>-in-script test fails against the previous plainindexOf()boundary. Restored after each.vp test runoverpages-page-data,pages-page-handler,pages-page-response,pages-isr-query-context,isr-cache,document,head, andscript-head-ordering.vp checkrepo-wide: no formatting, lint, or type errors.Full local suite and e2e were not run; leaving those to CI.
Risk / compatibility
onHeadReadyandcollectIsrHeadHTMLare optional internal parameters; omitting them preserves the previous behaviour exactly.data-next-head, so they refresh without invalidation.</head>is missing, the app overrides_document.getInitialProps, or the regeneration collects no head, the cached head is returned untouched.Non-goals
_document-rendered head children, CSS-in-JSstyles, or the head of apps that override_document.getInitialProps. All require running the full document pipeline on the regeneration path, which is a larger change than this one.