From 83f400e2af9a221565ca54374c13402099c3b2fa Mon Sep 17 00:00:00 2001 From: Visnu Pitiyanuvath Date: Thu, 1 Sep 2022 21:22:17 -0700 Subject: [PATCH] Match data fetch and busting cache key when path URI encodes (#39568) Fixes #38581 ## Bug - [x] Related issues linked using `fixes #number` - [x] Integration tests added - [ ] Errors have helpful link attached, see `contributing.md` ## Feature - [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR. - [ ] Related issues linked using `fixes #number` - [ ] Integration tests added - [ ] Documentation added - [ ] Telemetry added. In case of a feature if it's used or not. - [ ] Errors have helpful link attached, see `contributing.md` ## Documentation / Examples - [ ] Make sure the linting passes by running `pnpm lint` - [ ] The examples guidelines are followed from [our contributing doc](https://github.com/vercel/next.js/blob/canary/contributing.md#adding-examples) Co-authored-by: JJ Kasper --- packages/next/shared/lib/router/router.ts | 18 +++++----- .../dynamic-route-interpolation/index.test.ts | 36 ++++++++++++++++--- 2 files changed, 42 insertions(+), 12 deletions(-) diff --git a/packages/next/shared/lib/router/router.ts b/packages/next/shared/lib/router/router.ts index 93d6b285e164..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,10 +2064,6 @@ 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] } diff --git a/test/e2e/dynamic-route-interpolation/index.test.ts b/test/e2e/dynamic-route-interpolation/index.test.ts index 441a5a5ac3bb..874353991e6b 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}"))`) + 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}"))`) + await browser.close() + }) })