Skip to content

Commit 191acdf

Browse files
authored
Remove expired link resources via MutationObserver during development (vercel#48578)
Closes NEXT-684, closes vercel#43396. This PR implements a temporary workaround to address the issue that some browsers are always caching CSS resources during the lifetime of a session. We re-introduce the versioning query to the resource to avoid that, and then use Mutation Observer to do GC manually on the client. Once Float handles that by itself, we can probably remove this. Note that correctly handling GC here is **required** for correctness, not an optimization. That's why it took us a while to address this (even this PR is still a temporary workaround). Imagine that if you have: ```css h1 { color: red; } ``` and then you changed it to: ```css h1 { font-size: 300px; } ``` During HMR, if we don't remove the old resources but only insert the new one, both will be applied and you will still see the `<h1>` in red, which is wrong. Here's a recording of this PR working correctly in Firefox: https://user-images.githubusercontent.com/3676859/233132831-b88e4d8d-aec9-48c4-9aa7-7c7a149b377d.mp4
1 parent 9f5463d commit 191acdf

4 files changed

Lines changed: 76 additions & 15 deletions

File tree

packages/next/src/client/app-index.tsx

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,4 +296,56 @@ export function hydrate() {
296296
if (isError) {
297297
reactRoot.render(reactEl)
298298
}
299+
300+
// TODO-APP: Remove this logic when Float has GC built-in in development.
301+
if (process.env.NODE_ENV !== 'production') {
302+
const callback = (mutationList: MutationRecord[]) => {
303+
for (const mutation of mutationList) {
304+
if (mutation.type === 'childList') {
305+
for (const node of mutation.addedNodes) {
306+
if (
307+
'tagName' in node &&
308+
(node as HTMLLinkElement).tagName === 'LINK'
309+
) {
310+
const link = node as HTMLLinkElement
311+
if (link.dataset.precedence === 'next.js') {
312+
const href = link.getAttribute('href')
313+
if (href) {
314+
const [resource, version] = href.split('?v=')
315+
if (version) {
316+
const allLinks = document.querySelectorAll(
317+
`link[href^="${resource}"]`
318+
) as NodeListOf<HTMLLinkElement>
319+
for (const otherLink of allLinks) {
320+
if (otherLink.dataset.precedence === 'next.js') {
321+
const otherHref = otherLink.getAttribute('href')
322+
if (otherHref) {
323+
const [, otherVersion] = otherHref.split('?v=')
324+
if (!otherVersion || +otherVersion < +version) {
325+
otherLink.remove()
326+
const preloadLink = document.querySelector(
327+
`link[rel="preload"][as="style"][href="${otherHref}"]`
328+
)
329+
if (preloadLink) {
330+
preloadLink.remove()
331+
}
332+
}
333+
}
334+
}
335+
}
336+
}
337+
}
338+
}
339+
}
340+
}
341+
}
342+
}
343+
}
344+
345+
// Create an observer instance linked to the callback function
346+
const observer = new MutationObserver(callback)
347+
observer.observe(document.head, {
348+
childList: true,
349+
})
350+
}
299351
}

packages/next/src/server/app-render/app-render.tsx

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -379,12 +379,15 @@ export async function renderToHTMLOrFlight(
379379
? cssHrefs.map((href, index) => (
380380
<link
381381
rel="stylesheet"
382-
// In dev, Safari will wrongly cache the resource if you preload it:
382+
// In dev, Safari and Firefox will cache the resource during HMR:
383383
// - https://github.com/vercel/next.js/issues/5860
384384
// - https://bugs.webkit.org/show_bug.cgi?id=187726
385-
// We used to add a `?ts=` query for resources in `pages` to bypass it,
386-
// but in this case it is fine as we don't need to preload the styles.
387-
href={`${assetPrefix}/_next/${href}`}
385+
// Because of this, we add a `?v=` query to bypass the cache during
386+
// development. We need to also make sure that the number is always
387+
// increasing.
388+
href={`${assetPrefix}/_next/${href}${
389+
process.env.NODE_ENV === 'development' ? `?v=${Date.now()}` : ''
390+
}`}
388391
// @ts-ignore
389392
precedence={shouldPreload ? 'high' : undefined}
390393
key={index}
@@ -469,12 +472,15 @@ export async function renderToHTMLOrFlight(
469472
? stylesheets.map((href, index) => (
470473
<link
471474
rel="stylesheet"
472-
// In dev, Safari will wrongly cache the resource if you preload it:
475+
// In dev, Safari and Firefox will cache the resource during HMR:
473476
// - https://github.com/vercel/next.js/issues/5860
474477
// - https://bugs.webkit.org/show_bug.cgi?id=187726
475-
// We used to add a `?ts=` query for resources in `pages` to bypass it,
476-
// but in this case it is fine as we don't need to preload the styles.
477-
href={`${assetPrefix}/_next/${href}`}
478+
// Because of this, we add a `?v=` query to bypass the cache during
479+
// development. We need to also make sure that the number is always
480+
// increasing.
481+
href={`${assetPrefix}/_next/${href}${
482+
process.env.NODE_ENV === 'development' ? `?v=${Date.now()}` : ''
483+
}`}
478484
// `Precedence` is an opt-in signal for React to handle
479485
// resource loading and deduplication, etc:
480486
// https://github.com/facebook/react/pull/25060

test/e2e/app-dir/app-css/index.test.ts

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ createNextDescribe(
154154
const html = await next.render('/loading-bug/hi')
155155
// The link tag should be included together with loading
156156
expect(html).toMatch(
157-
/<link rel="stylesheet" href="(.+)\.css"\/><h2>Loading...<\/h2>/
157+
/<link rel="stylesheet" href="(.+)\.css(\?v=\d+)?"\/><h2>Loading...<\/h2>/
158158
)
159159
})
160160

@@ -233,8 +233,11 @@ createNextDescribe(
233233
it('should bundle css resources into chunks', async () => {
234234
const html = await next.render('/dashboard')
235235
expect(
236-
[...html.matchAll(/<link rel="stylesheet" href="[^.]+\.css"/g)]
237-
.length
236+
[
237+
...html.matchAll(
238+
/<link rel="stylesheet" href="[^.]+\.css(\?v=\d+)?"/g
239+
),
240+
].length
238241
).toBe(3)
239242
})
240243
})
@@ -280,14 +283,14 @@ createNextDescribe(
280283
const browser = await next.browser('/css/css-duplicate/a')
281284
expect(
282285
await browser.eval(
283-
`[...document.styleSheets].some(({ href }) => href.endsWith('/a/page.css'))`
286+
`[...document.styleSheets].some(({ href }) => href.includes('/a/page.css'))`
284287
)
285288
).toBe(true)
286289

287290
// Should not load the chunk from /b
288291
expect(
289292
await browser.eval(
290-
`[...document.styleSheets].some(({ href }) => href.endsWith('/b/page.css'))`
293+
`[...document.styleSheets].some(({ href }) => href.includes('/b/page.css'))`
291294
)
292295
).toBe(false)
293296
})

test/e2e/app-dir/app-css/middleware.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,11 @@ import { NextResponse } from 'next/server'
33
export async function middleware(request) {
44
// This middleware is used to test Suspensey CSS
55
if (
6-
request.url.endsWith('_next/static/css/app/suspensey-css/slow/page.css')
6+
request.url.includes('_next/static/css/app/suspensey-css/slow/page.css')
77
) {
88
await new Promise((resolve) => setTimeout(resolve, 150))
99
} else if (
10-
request.url.endsWith('_next/static/css/app/suspensey-css/timeout/page.css')
10+
request.url.includes('_next/static/css/app/suspensey-css/timeout/page.css')
1111
) {
1212
await new Promise((resolve) => setTimeout(resolve, 1000))
1313
}

0 commit comments

Comments
 (0)