From ed1c2fe700d4ac99410c4f15a69ca03df6179f29 Mon Sep 17 00:00:00 2001 From: Visnu Pitiyanuvath Date: Fri, 12 Aug 2022 21:56:54 -0700 Subject: [PATCH 1/4] Match data fetch and busting cache key when path URI encodes --- packages/next/shared/lib/router/router.ts | 1 + .../dynamic-route-interpolation/index.test.ts | 36 ++++++++++++++++--- 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/packages/next/shared/lib/router/router.ts b/packages/next/shared/lib/router/router.ts index 37b3f3b85b5d..9ffc9ff87ea2 100644 --- a/packages/next/shared/lib/router/router.ts +++ b/packages/next/shared/lib/router/router.ts @@ -2015,6 +2015,7 @@ export default class Router implements BaseRouter { : await fetchNextData({ dataHref: this.pageLoader.getDataHref({ href: formatWithValidation({ pathname, query }), + skipInterpolation: true, asPath: resolvedAs, locale, }), diff --git a/test/e2e/dynamic-route-interpolation/index.test.ts b/test/e2e/dynamic-route-interpolation/index.test.ts index 441a5a5ac3bb..9b376722fa16 100644 --- a/test/e2e/dynamic-route-interpolation/index.test.ts +++ b/test/e2e/dynamic-route-interpolation/index.test.ts @@ -2,6 +2,7 @@ import { createNext } from 'e2e-utils' import { NextInstance } from 'test/lib/next-modes/base' import { renderViaHTTP } from 'next-test-utils' import cheerio from 'cheerio' +import webdriver from 'next-webdriver' describe('Dynamic Route Interpolation', () => { let next: NextInstance @@ -10,14 +11,24 @@ describe('Dynamic Route Interpolation', () => { next = await createNext({ files: { 'pages/blog/[slug].js': ` + import Link from "next/link" + import { useRouter } from "next/router" + export function getServerSideProps({ params }) { - return { props: { slug: params.slug } } + return { props: { slug: params.slug, now: Date.now() } } } export default function Page(props) { - return

{props.slug}

+ const router = useRouter() + return ( +
+

{props.slug}

+ + {props.now} + +
+ ) } - `, 'pages/api/dynamic/[slug].js': ` @@ -25,7 +36,6 @@ describe('Dynamic Route Interpolation', () => { const { slug } = req.query res.end('slug: ' + slug) } - `, }, dependencies: {}, @@ -60,4 +70,22 @@ describe('Dynamic Route Interpolation', () => { const text = await renderViaHTTP(next.url, '/api/dynamic/[abc]') expect(text).toBe('slug: [abc]') }) + + it('should bust data cache', async () => { + const browser = await webdriver(next.url, '/blog/login') + await browser.elementById('now').click() // fetch data once + const text = await browser.elementById('now').text() + await browser.elementById('now').click() // fetch data again + await browser.waitForElementByCss(`#now:not(:text("${text}"))`, 100) + await browser.close() + }) + + it('should bust data cache with symbol', async () => { + const browser = await webdriver(next.url, '/blog/@login') + await browser.elementById('now').click() // fetch data once + const text = await browser.elementById('now').text() + await browser.elementById('now').click() // fetch data again + await browser.waitForElementByCss(`#now:not(:text("${text}"))`, 100) + await browser.close() + }) }) From 16735348f886e8c359e6a10d7f42386f2f1b728b Mon Sep 17 00:00:00 2001 From: Visnu Pitiyanuvath Date: Fri, 12 Aug 2022 22:15:15 -0700 Subject: [PATCH 2/4] fragment --- test/e2e/dynamic-route-interpolation/index.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/e2e/dynamic-route-interpolation/index.test.ts b/test/e2e/dynamic-route-interpolation/index.test.ts index 9b376722fa16..ffdd122f33f3 100644 --- a/test/e2e/dynamic-route-interpolation/index.test.ts +++ b/test/e2e/dynamic-route-interpolation/index.test.ts @@ -21,12 +21,12 @@ describe('Dynamic Route Interpolation', () => { export default function Page(props) { const router = useRouter() return ( -
+ <>

{props.slug}

{props.now} -
+ ) } `, From cda77fdc2aa2c13fe4bcdb3156d9cfa3858aa9ed Mon Sep 17 00:00:00 2001 From: Visnu Pitiyanuvath Date: Thu, 18 Aug 2022 21:31:40 -0700 Subject: [PATCH 3/4] Bust both cache keys --- packages/next/shared/lib/router/router.ts | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/packages/next/shared/lib/router/router.ts b/packages/next/shared/lib/router/router.ts index 9ffc9ff87ea2..50e5b87d4f16 100644 --- a/packages/next/shared/lib/router/router.ts +++ b/packages/next/shared/lib/router/router.ts @@ -2015,7 +2015,6 @@ export default class Router implements BaseRouter { : await fetchNextData({ dataHref: this.pageLoader.getDataHref({ href: formatWithValidation({ pathname, query }), - skipInterpolation: true, asPath: resolvedAs, locale, }), @@ -2053,11 +2052,21 @@ export default class Router implements BaseRouter { // middleware can skip cache per request with // x-middleware-cache: no-cache as well if (routeInfo.__N_SSP && fetchNextDataParams.dataHref) { - const cacheKey = new URL( - fetchNextDataParams.dataHref, - window.location.href - ).href - delete this.sdc[cacheKey] + // The data cache can exist under two similar keys: one with + // interpolation and one without + delete this.sdc[ + new URL(fetchNextDataParams.dataHref, window.location.href).href + ] + delete this.sdc[ + new URL( + this.pageLoader.getDataHref({ + href: formatWithValidation({ pathname, query }), + asPath: resolvedAs, + locale, + }), + window.location.href + ).href + ] } // we kick off a HEAD request in the background From 73e798877cab49d5e89d386bef583cf017afef62 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Thu, 1 Sep 2022 20:47:18 -0700 Subject: [PATCH 4/4] pass cacheKey down --- packages/next/shared/lib/router/router.ts | 30 +++++++------------ .../dynamic-route-interpolation/index.test.ts | 4 +-- 2 files changed, 13 insertions(+), 21 deletions(-) diff --git a/packages/next/shared/lib/router/router.ts b/packages/next/shared/lib/router/router.ts index d9231aa9d92f..5fab0820e1b6 100644 --- a/packages/next/shared/lib/router/router.ts +++ b/packages/next/shared/lib/router/router.ts @@ -497,6 +497,7 @@ function withMiddlewareEffects( getMiddlewareData(data.dataHref, data.response, options).then( (effect) => ({ dataHref: data.dataHref, + cacheKey: data.cacheKey, json: data.json, response: data.response, text: data.text, @@ -639,6 +640,7 @@ interface FetchDataOutput { json: Record | null response: Response text: string + cacheKey: string } interface FetchNextDataParams { @@ -680,7 +682,7 @@ function fetchNextData({ }) .then((response) => { if (response.ok && params?.method === 'HEAD') { - return { dataHref, response, text: '', json: {} } + return { dataHref, response, text: '', json: {}, cacheKey } } return response.text().then((text) => { @@ -695,7 +697,7 @@ function fetchNextData({ hasMiddleware && [301, 302, 307, 308].includes(response.status) ) { - return { dataHref, response, text, json: {} } + return { dataHref, response, text, json: {}, cacheKey } } if (!hasMiddleware && response.status === 404) { @@ -705,6 +707,7 @@ function fetchNextData({ json: { notFound: SSG_DATA_NOT_FOUND }, response, text, + cacheKey, } } } @@ -728,6 +731,7 @@ function fetchNextData({ json: parseJSON ? tryToParseAsJSON(text) : null, response, text, + cacheKey, } }) }) @@ -2014,9 +2018,9 @@ export default class Router implements BaseRouter { const shouldFetchData = routeInfo.__N_SSG || routeInfo.__N_SSP || routeInfo.__N_RSC - const { props } = await this._getData(async () => { + const { props, cacheKey } = await this._getData(async () => { if (shouldFetchData && !useStreamedFlightData) { - const { json } = data?.json + const { json, cacheKey: _cacheKey } = data?.json ? data : await fetchNextData({ dataHref: this.pageLoader.getDataHref({ @@ -2033,12 +2037,14 @@ export default class Router implements BaseRouter { }) return { + cacheKey: _cacheKey, props: json || {}, } } return { headers: {}, + cacheKey: '', props: await this.getInitialProps( routeInfo.Component, // we provide AppTree later so this needs to be `any` @@ -2058,21 +2064,7 @@ export default class Router implements BaseRouter { // middleware can skip cache per request with // x-middleware-cache: no-cache as well if (routeInfo.__N_SSP && fetchNextDataParams.dataHref) { - // The data cache can exist under two similar keys: one with - // interpolation and one without - delete this.sdc[ - new URL(fetchNextDataParams.dataHref, window.location.href).href - ] - delete this.sdc[ - new URL( - this.pageLoader.getDataHref({ - href: formatWithValidation({ pathname, query }), - asPath: resolvedAs, - locale, - }), - window.location.href - ).href - ] + delete this.sdc[cacheKey] } // we kick off a HEAD request in the background diff --git a/test/e2e/dynamic-route-interpolation/index.test.ts b/test/e2e/dynamic-route-interpolation/index.test.ts index ffdd122f33f3..874353991e6b 100644 --- a/test/e2e/dynamic-route-interpolation/index.test.ts +++ b/test/e2e/dynamic-route-interpolation/index.test.ts @@ -76,7 +76,7 @@ describe('Dynamic Route Interpolation', () => { await browser.elementById('now').click() // fetch data once const text = await browser.elementById('now').text() await browser.elementById('now').click() // fetch data again - await browser.waitForElementByCss(`#now:not(:text("${text}"))`, 100) + await browser.waitForElementByCss(`#now:not(:text("${text}"))`) await browser.close() }) @@ -85,7 +85,7 @@ describe('Dynamic Route Interpolation', () => { await browser.elementById('now').click() // fetch data once const text = await browser.elementById('now').text() await browser.elementById('now').click() // fetch data again - await browser.waitForElementByCss(`#now:not(:text("${text}"))`, 100) + await browser.waitForElementByCss(`#now:not(:text("${text}"))`) await browser.close() }) })