Skip to content

Commit

Permalink
Match data fetch and busting cache key when path URI encodes (#39568)
Browse files Browse the repository at this point in the history
Fixes #38581
<!--
Thanks for opening a PR! Your contribution is much appreciated.
In order to make sure your PR is handled as smoothly as possible we request that you follow the checklist sections below.
Choose the right checklist for the change that you're making:
-->

## 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 <jj@jjsweb.site>
  • Loading branch information
visnup and ijjk committed Sep 2, 2022
1 parent b9238ec commit abcf991
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 12 deletions.
18 changes: 10 additions & 8 deletions packages/next/shared/lib/router/router.ts
Expand Up @@ -497,6 +497,7 @@ function withMiddlewareEffects<T extends FetchDataOutput>(
getMiddlewareData(data.dataHref, data.response, options).then(
(effect) => ({
dataHref: data.dataHref,
cacheKey: data.cacheKey,
json: data.json,
response: data.response,
text: data.text,
Expand Down Expand Up @@ -639,6 +640,7 @@ interface FetchDataOutput {
json: Record<string, any> | null
response: Response
text: string
cacheKey: string
}

interface FetchNextDataParams {
Expand Down Expand Up @@ -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) => {
Expand All @@ -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) {
Expand All @@ -705,6 +707,7 @@ function fetchNextData({
json: { notFound: SSG_DATA_NOT_FOUND },
response,
text,
cacheKey,
}
}
}
Expand All @@ -728,6 +731,7 @@ function fetchNextData({
json: parseJSON ? tryToParseAsJSON(text) : null,
response,
text,
cacheKey,
}
})
})
Expand Down Expand Up @@ -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({
Expand All @@ -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`
Expand All @@ -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]
}

Expand Down
36 changes: 32 additions & 4 deletions test/e2e/dynamic-route-interpolation/index.test.ts
Expand Up @@ -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
Expand All @@ -10,22 +11,31 @@ 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 <p id="slug">{props.slug}</p>
const router = useRouter()
return (
<>
<p id="slug">{props.slug}</p>
<Link href={router.asPath}>
<a id="now">{props.now}</a>
</Link>
</>
)
}
`,

'pages/api/dynamic/[slug].js': `
export default function Page(req, res) {
const { slug } = req.query
res.end('slug: ' + slug)
}
`,
},
dependencies: {},
Expand Down Expand Up @@ -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()
})
})

0 comments on commit abcf991

Please sign in to comment.