From 8c3a67e0ca3f16f80e3a483c76aa33d400b2fad8 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Wed, 25 May 2022 21:09:00 -0500 Subject: [PATCH 1/2] Update to not trigger revalidation during prefetch --- packages/next/server/base-server.ts | 1 + packages/next/server/response-cache.ts | 3 +- packages/next/shared/lib/router/router.ts | 74 +++++++++------ .../app/pages/blog/[slug].js | 26 ++++++ .../prerender-prefetch/app/pages/index.js | 28 ++++++ .../prerender-prefetch/index.test.ts | 91 +++++++++++++++++++ 6 files changed, 193 insertions(+), 30 deletions(-) create mode 100644 test/production/prerender-prefetch/app/pages/blog/[slug].js create mode 100644 test/production/prerender-prefetch/app/pages/index.js create mode 100644 test/production/prerender-prefetch/index.test.ts diff --git a/packages/next/server/base-server.ts b/packages/next/server/base-server.ts index 0e7d48c52c05a..0a8a3a9504500 100644 --- a/packages/next/server/base-server.ts +++ b/packages/next/server/base-server.ts @@ -1586,6 +1586,7 @@ export default abstract class Server { }, { isManualRevalidate, + isPrefetch: req.headers.purpose === 'prefetch', } ) diff --git a/packages/next/server/response-cache.ts b/packages/next/server/response-cache.ts index 864c960e44a04..b2acbbdbf8be9 100644 --- a/packages/next/server/response-cache.ts +++ b/packages/next/server/response-cache.ts @@ -99,6 +99,7 @@ export default class ResponseCache { responseGenerator: ResponseGenerator, context: { isManualRevalidate?: boolean + isPrefetch?: boolean } ): Promise { // ensure manual revalidate doesn't block normal requests @@ -175,7 +176,7 @@ export default class ResponseCache { } : cachedResponse.value, }) - if (!cachedResponse.isStale) { + if (!cachedResponse.isStale || context.isPrefetch) { // The cached value is still valid, so we don't need // to update it yet. return diff --git a/packages/next/shared/lib/router/router.ts b/packages/next/shared/lib/router/router.ts index 96d4f37d542a5..120fb7a6a736b 100644 --- a/packages/next/shared/lib/router/router.ts +++ b/packages/next/shared/lib/router/router.ts @@ -535,7 +535,7 @@ const SSG_DATA_NOT_FOUND = Symbol('SSG_DATA_NOT_FOUND') function fetchRetry( url: string, attempts: number, - opts: { text?: boolean } + opts: { text?: boolean; isPrefetch?: boolean; method?: string } ): Promise { return fetch(url, { // Cookies are required to be present for Next.js' SSG "Preview Mode". @@ -550,6 +550,12 @@ function fetchRetry( // > option instead of relying on the default. // https://github.com/github/fetch#caveats credentials: 'same-origin', + method: opts.method || 'GET', + headers: opts.isPrefetch + ? { + purpose: 'prefetch', + } + : {}, }).then((res) => { if (!res.ok) { if (attempts > 1 && res.status >= 500) { @@ -574,38 +580,46 @@ function fetchNextData( isServerRender: boolean, text: boolean | undefined, inflightCache: NextDataCache, - persistCache: boolean + persistCache: boolean, + isPrefetch: boolean ) { const { href: cacheKey } = new URL(dataHref, window.location.href) + const getData = (background = false) => + fetchRetry(dataHref, isServerRender ? 3 : 1, { + text, + isPrefetch, + method: background ? 'HEAD' : 'GET', + }) + .catch((err: Error) => { + // We should only trigger a server-side transition if this was caused + // on a client-side transition. Otherwise, we'd get into an infinite + // loop. + + if (!isServerRender) { + markAssetError(err) + } + throw err + }) + .then((data) => { + if (!persistCache || process.env.NODE_ENV !== 'production') { + delete inflightCache[cacheKey] + } + return data + }) + .catch((err) => { + delete inflightCache[cacheKey] + throw err + }) if (inflightCache[cacheKey] !== undefined) { + // we kick off a HEAD request in the background + // when a non-prefetch request is made to signal revalidation + if (!isPrefetch && persistCache) { + getData(true).catch(() => {}) + } return inflightCache[cacheKey] } - return (inflightCache[cacheKey] = fetchRetry( - dataHref, - isServerRender ? 3 : 1, - { text } - ) - .catch((err: Error) => { - // We should only trigger a server-side transition if this was caused - // on a client-side transition. Otherwise, we'd get into an infinite - // loop. - - if (!isServerRender) { - markAssetError(err) - } - throw err - }) - .then((data) => { - if (!persistCache || process.env.NODE_ENV !== 'production') { - delete inflightCache[cacheKey] - } - return data - }) - .catch((err) => { - delete inflightCache[cacheKey] - throw err - })) + return (inflightCache[cacheKey] = getData()) } interface NextDataCache { @@ -1617,7 +1631,8 @@ export default class Router implements BaseRouter { this.isSsr, false, __N_SSG ? this.sdc : this.sdr, - !!__N_SSG && !isPreview + !!__N_SSG && !isPreview, + false ) : this.getInitialProps( Component, @@ -1851,6 +1866,7 @@ export default class Router implements BaseRouter { false, false, // text this.sdc, + !this.isPreview, true ) : false @@ -1915,7 +1931,7 @@ export default class Router implements BaseRouter { _getFlightData(dataHref: string): Promise { // Do not cache RSC flight response since it's not a static resource - return fetchNextData(dataHref, true, true, this.sdc, false).then( + return fetchNextData(dataHref, true, true, this.sdc, false, false).then( (serialized) => { return { data: serialized } } diff --git a/test/production/prerender-prefetch/app/pages/blog/[slug].js b/test/production/prerender-prefetch/app/pages/blog/[slug].js new file mode 100644 index 0000000000000..4c246aa6b28bd --- /dev/null +++ b/test/production/prerender-prefetch/app/pages/blog/[slug].js @@ -0,0 +1,26 @@ +export default function Page(props) { + return ( + <> +

blog/[slug]

+

{JSON.stringify(props)}

+ + ) +} + +export function getStaticProps({ params }) { + console.log('revalidating /blog', params.slug) + return { + props: { + params, + now: Date.now(), + }, + revalidate: 2, + } +} + +export function getStaticPaths() { + return { + paths: ['/blog/first', '/blog/second'], + fallback: false, + } +} diff --git a/test/production/prerender-prefetch/app/pages/index.js b/test/production/prerender-prefetch/app/pages/index.js new file mode 100644 index 0000000000000..1d440965f0a24 --- /dev/null +++ b/test/production/prerender-prefetch/app/pages/index.js @@ -0,0 +1,28 @@ +import Link from 'next/link' + +export default function Page(props) { + return ( + <> +

index

+

{JSON.stringify(props)}

+ + /blog/first + +
+ + /blog/second + +
+ + ) +} + +export function getStaticProps() { + console.log('revalidating /') + return { + props: { + now: Date.now(), + }, + revalidate: 1, + } +} diff --git a/test/production/prerender-prefetch/index.test.ts b/test/production/prerender-prefetch/index.test.ts new file mode 100644 index 0000000000000..5e20a50a65171 --- /dev/null +++ b/test/production/prerender-prefetch/index.test.ts @@ -0,0 +1,91 @@ +import { NextInstance } from 'test/lib/next-modes/base' +import { createNext, FileRef } from 'e2e-utils' +import { check, fetchViaHTTP, waitFor } from 'next-test-utils' +import cheerio from 'cheerio' +import { join } from 'path' +import webdriver from 'next-webdriver' +import assert from 'assert' + +describe('Prerender prefetch', () => { + let next: NextInstance + + beforeAll(async () => { + next = await createNext({ + files: { + pages: new FileRef(join(__dirname, 'app/pages')), + }, + dependencies: {}, + }) + }) + afterAll(() => next.destroy()) + + it('should not revalidate during prefetching', async () => { + const reqs = {} + + // get initial values + for (const path of ['/blog/first', '/blog/second']) { + const res = await fetchViaHTTP(next.url, path) + expect(res.status).toBe(200) + + const $ = cheerio.load(await res.text()) + const props = JSON.parse($('#props').text()) + reqs[path] = props + } + + const browser = await webdriver(next.url, '/') + + // wait for prefetch to occur + await check(async () => { + const cache = await browser.eval('JSON.stringify(window.next.router.sdc)') + return cache.includes('/blog/first') && cache.includes('/blog/second') + ? 'success' + : cache + }, 'success') + + await waitFor(3000) + await browser.refresh() + + // reload after revalidate period and wait for prefetch again + await check(async () => { + const cache = await browser.eval('JSON.stringify(window.next.router.sdc)') + return cache.includes('/blog/first') && cache.includes('/blog/second') + ? 'success' + : cache + }, 'success') + + // ensure revalidate did not occur from prefetch + for (const path of ['/blog/first', '/blog/second']) { + const res = await fetchViaHTTP(next.url, path) + expect(res.status).toBe(200) + + const $ = cheerio.load(await res.text()) + const props = JSON.parse($('#props').text()) + expect(props).toEqual(reqs[path]) + } + }) + + it('should trigger revalidation after navigation', async () => { + const getData = () => + fetchViaHTTP( + next.url, + `/_next/data/${next.buildId}/blog/first.json`, + undefined, + { + headers: { + purpose: 'prefetch', + }, + } + ) + const initialDataRes = await getData() + const initialData = await initialDataRes.json() + const browser = await webdriver(next.url, '/') + + await browser.elementByCss('#to-blog-first').click() + + await check(async () => { + const data = await getData() + assert.notDeepEqual(initialData, data) + return 'success' + }, 'success') + }) +}) From be49b4bedbb3e15c349d706686daf9ff6fa4d20d Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Wed, 25 May 2022 21:50:23 -0500 Subject: [PATCH 2/2] handle test case --- packages/next/shared/lib/router/router.ts | 15 ++++++++++++--- .../preload-viewport/test/index.test.js | 2 +- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/packages/next/shared/lib/router/router.ts b/packages/next/shared/lib/router/router.ts index 120fb7a6a736b..411d2676f4bb9 100644 --- a/packages/next/shared/lib/router/router.ts +++ b/packages/next/shared/lib/router/router.ts @@ -571,10 +571,15 @@ function fetchRetry( } throw new Error(`Failed to load static props`) } - return opts.text ? res.text() : res.json() + + if (opts.method !== 'HEAD') { + return opts.text ? res.text() : res.json() + } }) } +const backgroundCache: Record> = {} + function fetchNextData( dataHref: string, isServerRender: boolean, @@ -614,8 +619,12 @@ function fetchNextData( if (inflightCache[cacheKey] !== undefined) { // we kick off a HEAD request in the background // when a non-prefetch request is made to signal revalidation - if (!isPrefetch && persistCache) { - getData(true).catch(() => {}) + if (!isPrefetch && persistCache && !backgroundCache[cacheKey]) { + backgroundCache[cacheKey] = getData(true) + .catch(() => {}) + .then(() => { + delete backgroundCache[cacheKey] + }) } return inflightCache[cacheKey] } diff --git a/test/integration/preload-viewport/test/index.test.js b/test/integration/preload-viewport/test/index.test.js index 9946d991b38fd..b8aad8d5a45fb 100644 --- a/test/integration/preload-viewport/test/index.test.js +++ b/test/integration/preload-viewport/test/index.test.js @@ -67,7 +67,7 @@ describe('Prefetching Links in viewport', () => { expect( nextDataRequests.filter((reqUrl) => reqUrl.includes('/ssg/slow.json')) .length - ).toBe(1) + ).toBe(2) }) it('should handle timed out prefetch correctly', async () => {