Skip to content

Commit fbc7684

Browse files
lukesandberglukesandberg
committed
Scope Safari ?ts= cache-buster to CSS/font assets only (Pages Router) (#92580)
### What? Refactors the Safari `?ts=` cache-busting workaround in the Pages Router so it **only appears on CSS and font URLs**, not on script tags or script preload links. ### Why? The `?ts=` timestamp was originally added to all preloaded assets as a workaround for a [Safari caching bug](https://bugs.webkit.org/show_bug.cgi?id=187726) (see [#5860](#5860)). When it appears on `<script>` tags, the Turbopack runtime's `getAssetSuffixFromScriptSrc()` reads the executing script's query string and infers it as the `ASSET_SUFFIX`, which then leaks onto all static asset URLs — including images. This causes `next/image` validation errors because the image URL gets an unexpected `?ts=` parameter. Fixes #92118 ### How? Instead of maintaining parallel `assetQueryString` (with `?ts=`) and `scriptAssetQueryString` (without `?ts=`) paths, this PR: 1. **`assetQueryString`** carries only the deployment token (`?dpl=...`) and is used for all script-related URLs (script tags, script preloads) 2. **`safariCacheBuster`** is a new, separate field (`?ts=<timestamp>` or `""`) that is only combined with `assetQueryString` at the 3 CSS/font URL sites via a `joinQueryStrings()` helper 3. Removes the `scriptAssetQueryString` / `scriptMutableAssetQueryString` / `cssAssetQueryString` fields entirely The Safari cache-buster is computed the same way as before (dev server only, Safari user-agent check) — it just no longer contaminates the base query string. ### Test plan - [x] Updated existing test in `test/e2e/app-document/rendering.test.ts` to assert `?ts=` appears only on `<link rel="preload" as="style">` and `<link rel="preload" as="font">`, and does NOT appear on `<script>` or `<link rel="preload" as="script">` - [x] `pnpm --filter=next types` passes <!-- NEXT_JS_LLM_PR -->
1 parent 805d758 commit fbc7684

5 files changed

Lines changed: 66 additions & 42 deletions

File tree

‎packages/next/src/pages/_document.tsx‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -384,7 +384,7 @@ export class Head extends React.Component<HeadProps> {
384384
getCssLinks(files: DocumentFiles): JSX.Element[] | null {
385385
const {
386386
assetPrefix,
387-
assetQueryString,
387+
cssAssetQueryString,
388388
dynamicImports,
389389
dynamicCssManifest,
390390
crossOrigin,
@@ -422,7 +422,7 @@ export class Head extends React.Component<HeadProps> {
422422
rel="preload"
423423
href={`${assetPrefix}/_next/${encodeURIPath(
424424
file
425-
)}${assetQueryString}`}
425+
)}${cssAssetQueryString}`}
426426
as="style"
427427
crossOrigin={this.props.crossOrigin || crossOrigin}
428428
/>
@@ -436,7 +436,7 @@ export class Head extends React.Component<HeadProps> {
436436
rel="stylesheet"
437437
href={`${assetPrefix}/_next/${encodeURIPath(
438438
file
439-
)}${assetQueryString}`}
439+
)}${cssAssetQueryString}`}
440440
crossOrigin={this.props.crossOrigin || crossOrigin}
441441
data-n-g={isUnmanagedFile ? undefined : isSharedFile ? '' : undefined}
442442
data-n-p={
@@ -589,7 +589,7 @@ export class Head extends React.Component<HeadProps> {
589589
optimizeCss,
590590
assetPrefix,
591591
nextFontManifest,
592-
assetQueryString,
592+
cssAssetQueryString,
593593
} = this.context
594594

595595
const disableRuntimeJS = unstable_runtimeJS === false
@@ -659,7 +659,7 @@ export class Head extends React.Component<HeadProps> {
659659
nextFontManifest,
660660
dangerousAsPath,
661661
assetPrefix,
662-
assetQueryString
662+
cssAssetQueryString
663663
)
664664

665665
const tracingMetadata = getTracedMetadata(

‎packages/next/src/server/render-result.ts‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,6 @@ export type AppPageRenderResultMetadata = {
6767
export type PagesRenderResultMetadata = {
6868
pageData?: any
6969
cacheControl?: CacheControl
70-
assetQueryString?: string
71-
mutableAssetQueryString?: string
7270
isNotFound?: boolean
7371
isRedirect?: boolean
7472
}

‎packages/next/src/server/render.tsx‎

Lines changed: 35 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,6 @@ export type RenderOptsPartial = {
254254
optimizeCss: any
255255
nextConfigOutput?: 'standalone' | 'export'
256256
nextScriptWorkers: any
257-
assetQueryString?: string
258257
resolvedUrl?: string
259258
resolvedAsPath?: string
260259
setIsrStatus?: (key: string, value: boolean | undefined) => void
@@ -445,6 +444,16 @@ function serializeError(
445444
}
446445
}
447446

447+
function getSafariCacheBusterQueryString(req: IncomingMessage): string {
448+
if (process.env.__NEXT_DEV_SERVER) {
449+
const userAgent = (req.headers['user-agent'] || '').toLowerCase()
450+
if (userAgent.includes('safari') && !userAgent.includes('chrome')) {
451+
return `?ts=${Date.now()}`
452+
}
453+
}
454+
return ''
455+
}
456+
448457
export async function renderToHTMLImpl(
449458
req: IncomingMessage,
450459
res: ServerResponse,
@@ -458,35 +467,30 @@ export async function renderToHTMLImpl(
458467
// Adds support for reading `cookies` in `getServerSideProps` when SSR.
459468
setLazyProp({ req: req as any }, 'cookies', getCookieParser(req.headers))
460469

461-
let baseAssetQueryString =
462-
(process.env.__NEXT_DEV_SERVER && renderOpts.assetQueryString) || ''
463-
464-
if (process.env.__NEXT_DEV_SERVER && !baseAssetQueryString) {
465-
const userAgent = (req.headers['user-agent'] || '').toLowerCase()
466-
if (userAgent.includes('safari') && !userAgent.includes('chrome')) {
467-
// In dev we invalidate the cache by appending a timestamp to the resource URL.
468-
// This is a workaround to fix https://github.com/vercel/next.js/issues/5860
469-
// TODO: remove this workaround when https://bugs.webkit.org/show_bug.cgi?id=187726 is fixed.
470-
// Note: The workaround breaks breakpoints on reload since the script url always changes,
471-
// so we only apply it to Safari.
472-
baseAssetQueryString = `?ts=${Date.now()}`
473-
}
474-
}
475-
476-
const mutableAssetQueryString =
477-
baseAssetQueryString +
478-
(sharedContext.deploymentId
479-
? `${baseAssetQueryString ? '&' : '?'}dpl=${sharedContext.deploymentId}`
480-
: '')
481-
const assetQueryString =
482-
baseAssetQueryString +
470+
// cssCacheBuster is a workaround for a Safari bug
471+
// (https://bugs.webkit.org/show_bug.cgi?id=187726) where preloaded CSS
472+
// resources are cached and not re-fetched on HMR. It must only be applied
473+
// to CSS and font assets — not to script tags — because the Turbopack
474+
// runtime infers ASSET_SUFFIX from the executing script's query string and
475+
// leaks it onto all static asset URLs (including images), causing
476+
// next/image validation errors.
477+
// See https://github.com/vercel/next.js/issues/92118.
478+
const cssCacheBuster = getSafariCacheBusterQueryString(req)
479+
480+
const mutableAssetQueryString = sharedContext.deploymentId
481+
? `?dpl=${sharedContext.deploymentId}`
482+
: ''
483+
const assetQueryString = sharedContext.clientAssetToken
484+
? `?dpl=${sharedContext.clientAssetToken}`
485+
: ''
486+
// cssAssetQueryString is assetQueryString with the cacheBuster prepended.
487+
// Use this for CSS and font URLs; use assetQueryString for script URLs.
488+
const cssAssetQueryString =
489+
cssCacheBuster +
483490
(sharedContext.clientAssetToken
484-
? `${baseAssetQueryString ? '&' : '?'}dpl=${sharedContext.clientAssetToken}`
491+
? `${cssCacheBuster ? '&' : '?'}dpl=${sharedContext.clientAssetToken}`
485492
: '')
486-
const metadata: PagesRenderResultMetadata = {
487-
assetQueryString,
488-
mutableAssetQueryString,
489-
}
493+
const metadata: PagesRenderResultMetadata = {}
490494

491495
// don't modify original query object
492496
query = Object.assign({}, query)
@@ -1530,8 +1534,9 @@ export async function renderToHTMLImpl(
15301534
? pageConfig.unstable_runtimeJS
15311535
: undefined,
15321536
unstable_JsPreload: pageConfig.unstable_JsPreload,
1533-
assetQueryString: assetQueryString || '',
1534-
mutableAssetQueryString: mutableAssetQueryString || '',
1537+
assetQueryString,
1538+
cssAssetQueryString,
1539+
mutableAssetQueryString,
15351540
scriptLoader,
15361541
locale,
15371542
disableOptimizedLoading,

‎packages/next/src/shared/lib/html-context.shared-runtime.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,11 @@ export type HtmlProps = {
3131
unstable_JsPreload?: false
3232
assetQueryString: string
3333
mutableAssetQueryString: string
34+
/**
35+
* Asset query string for CSS and font assets.
36+
* See https://github.com/vercel/next.js/issues/92118.
37+
*/
38+
cssAssetQueryString: string
3439
scriptLoader: {
3540
afterInteractive?: string[]
3641
beforeInteractive?: any[]

‎test/e2e/app-document/rendering.test.ts‎

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -81,19 +81,35 @@ describe('Document and App - Rendering via HTTP', () => {
8181
})
8282

8383
if (isNextDev) {
84-
// This is a workaround to fix https://github.com/vercel/next.js/issues/5860
85-
// TODO: remove this workaround when https://bugs.webkit.org/show_bug.cgi?id=187726 is fixed.
86-
it('adds a timestamp to link tags with preload attribute to invalidate the cache in dev', async () => {
84+
// The ?ts= timestamp is a workaround for a Safari preload cache bug:
85+
// https://github.com/vercel/next.js/issues/5860
86+
// https://bugs.webkit.org/show_bug.cgi?id=187726
87+
// It must only appear on CSS/font resources, not on script tags, because
88+
// the Turbopack runtime reads ASSET_SUFFIX from the executing script's
89+
// query string and would leak it onto image URLs.
90+
it('adds a timestamp only to CSS/font link tags to invalidate the cache in dev', async () => {
8791
const $ = await next.render$('/', undefined, {
8892
headers: { 'user-agent': 'Safari' },
8993
})
90-
$('link[rel=preload]').each((index, element) => {
94+
// CSS preload links must have ?ts= for Safari cache busting
95+
$('link[rel=preload][as=style]').each((index, element) => {
9196
const href = $(element).attr('href')
9297
expect(href).toMatch(/^[^?]+\?ts=\d+$/)
9398
})
99+
// Font preload links must have ?ts= for Safari cache busting
100+
$('link[rel=preload][as=font]').each((index, element) => {
101+
const href = $(element).attr('href')
102+
expect(href).toMatch(/^[^?]+\?ts=\d+$/)
103+
})
104+
// Script preload links must NOT have ?ts= (Turbopack ASSET_SUFFIX bug)
105+
$('link[rel=preload][as=script]').each((index, element) => {
106+
const src = $(element).attr('href')
107+
expect(src).not.toMatch(/[?&]ts=/)
108+
})
109+
// Script tags must NOT have ?ts= (Turbopack ASSET_SUFFIX bug)
94110
$('script[src]').each((index, element) => {
95111
const src = $(element).attr('src')
96-
expect(src).toMatch(/^[^?]+\?ts=\d+$/)
112+
expect(src).not.toMatch(/[?&]ts=/)
97113
})
98114
})
99115
}

0 commit comments

Comments
 (0)