Skip to content
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

Remove expired link resources via MutationObserver during development #48578

Merged
merged 3 commits into from
Apr 19, 2023
Merged
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
52 changes: 52 additions & 0 deletions packages/next/src/client/app-index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -296,4 +296,56 @@ export function hydrate() {
if (isError) {
reactRoot.render(reactEl)
}

// TODO-APP: Remove this logic when Float has GC built-in in development.
if (process.env.NODE_ENV !== 'production') {
const callback = (mutationList: MutationRecord[]) => {
for (const mutation of mutationList) {
if (mutation.type === 'childList') {
for (const node of mutation.addedNodes) {
if (
'tagName' in node &&
(node as HTMLLinkElement).tagName === 'LINK'
) {
const link = node as HTMLLinkElement
if (link.dataset.precedence === 'next.js') {
const href = link.getAttribute('href')
if (href) {
const [resource, version] = href.split('?v=')
if (version) {
const allLinks = document.querySelectorAll(
`link[href^="${resource}"]`
) as NodeListOf<HTMLLinkElement>
for (const otherLink of allLinks) {
if (otherLink.dataset.precedence === 'next.js') {
const otherHref = otherLink.getAttribute('href')
if (otherHref) {
const [, otherVersion] = otherHref.split('?v=')
if (!otherVersion || +otherVersion < +version) {
otherLink.remove()
const preloadLink = document.querySelector(
`link[rel="preload"][as="style"][href="${otherHref}"]`
)
if (preloadLink) {
preloadLink.remove()
}
}
}
}
}
}
}
}
}
}
}
}
}

// Create an observer instance linked to the callback function
const observer = new MutationObserver(callback)
observer.observe(document.head, {
childList: true,
})
}
}
22 changes: 14 additions & 8 deletions packages/next/src/server/app-render/app-render.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -379,12 +379,15 @@ export async function renderToHTMLOrFlight(
? cssHrefs.map((href, index) => (
<link
rel="stylesheet"
// In dev, Safari will wrongly cache the resource if you preload it:
// In dev, Safari and Firefox will cache the resource during HMR:
// - https://github.com/vercel/next.js/issues/5860
// - https://bugs.webkit.org/show_bug.cgi?id=187726
// We used to add a `?ts=` query for resources in `pages` to bypass it,
// but in this case it is fine as we don't need to preload the styles.
href={`${assetPrefix}/_next/${href}`}
// Because of this, we add a `?v=` query to bypass the cache during
// development. We need to also make sure that the number is always
// increasing.
href={`${assetPrefix}/_next/${href}${
process.env.NODE_ENV === 'development' ? `?v=${Date.now()}` : ''
}`}
// @ts-ignore
precedence={shouldPreload ? 'high' : undefined}
key={index}
Expand Down Expand Up @@ -469,12 +472,15 @@ export async function renderToHTMLOrFlight(
? stylesheets.map((href, index) => (
<link
rel="stylesheet"
// In dev, Safari will wrongly cache the resource if you preload it:
// In dev, Safari and Firefox will cache the resource during HMR:
// - https://github.com/vercel/next.js/issues/5860
// - https://bugs.webkit.org/show_bug.cgi?id=187726
// We used to add a `?ts=` query for resources in `pages` to bypass it,
// but in this case it is fine as we don't need to preload the styles.
href={`${assetPrefix}/_next/${href}`}
// Because of this, we add a `?v=` query to bypass the cache during
// development. We need to also make sure that the number is always
// increasing.
href={`${assetPrefix}/_next/${href}${
process.env.NODE_ENV === 'development' ? `?v=${Date.now()}` : ''
}`}
// `Precedence` is an opt-in signal for React to handle
// resource loading and deduplication, etc:
// https://github.com/facebook/react/pull/25060
Expand Down
13 changes: 8 additions & 5 deletions test/e2e/app-dir/app-css/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ createNextDescribe(
const html = await next.render('/loading-bug/hi')
// The link tag should be included together with loading
expect(html).toMatch(
/<link rel="stylesheet" href="(.+)\.css"\/><h2>Loading...<\/h2>/
/<link rel="stylesheet" href="(.+)\.css(\?v=\d+)?"\/><h2>Loading...<\/h2>/
)
})

Expand Down Expand Up @@ -233,8 +233,11 @@ createNextDescribe(
it('should bundle css resources into chunks', async () => {
const html = await next.render('/dashboard')
expect(
[...html.matchAll(/<link rel="stylesheet" href="[^.]+\.css"/g)]
.length
[
...html.matchAll(
/<link rel="stylesheet" href="[^.]+\.css(\?v=\d+)?"/g
),
].length
).toBe(3)
})
})
Expand Down Expand Up @@ -280,14 +283,14 @@ createNextDescribe(
const browser = await next.browser('/css/css-duplicate/a')
expect(
await browser.eval(
`[...document.styleSheets].some(({ href }) => href.endsWith('/a/page.css'))`
`[...document.styleSheets].some(({ href }) => href.includes('/a/page.css'))`
)
).toBe(true)

// Should not load the chunk from /b
expect(
await browser.eval(
`[...document.styleSheets].some(({ href }) => href.endsWith('/b/page.css'))`
`[...document.styleSheets].some(({ href }) => href.includes('/b/page.css'))`
)
).toBe(false)
})
Expand Down
4 changes: 2 additions & 2 deletions test/e2e/app-dir/app-css/middleware.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@ import { NextResponse } from 'next/server'
export async function middleware(request) {
// This middleware is used to test Suspensey CSS
if (
request.url.endsWith('_next/static/css/app/suspensey-css/slow/page.css')
request.url.includes('_next/static/css/app/suspensey-css/slow/page.css')
) {
await new Promise((resolve) => setTimeout(resolve, 150))
} else if (
request.url.endsWith('_next/static/css/app/suspensey-css/timeout/page.css')
request.url.includes('_next/static/css/app/suspensey-css/timeout/page.css')
) {
await new Promise((resolve) => setTimeout(resolve, 1000))
}
Expand Down