Skip to content

Commit

Permalink
Fix revalidate for initial notFound: true paths (#28097)
Browse files Browse the repository at this point in the history
This fixes revalidation not occurring correctly when `notFound: true` is returned during build, additional tests have been added to ensure this is working correctly for dynamic and non-dynamic pages returning `notFound: true` during build and then revalidating afterwards.  

## Bug

- [x] Related issues linked using `fixes #number`
- [x] Integration tests added
- [x] Errors have helpful link attached, see `contributing.md`

Fixes: #21453
  • Loading branch information
ijjk committed Aug 14, 2021
1 parent 8a80b0b commit 1697852
Show file tree
Hide file tree
Showing 7 changed files with 128 additions and 8 deletions.
7 changes: 6 additions & 1 deletion packages/next/server/incremental-cache.ts
Expand Up @@ -139,7 +139,12 @@ export class IncrementalCache {
// let's check the disk for seed data
if (!data) {
if (this.prerenderManifest.notFoundRoutes.includes(pathname)) {
return { revalidateAfter: false, value: null }
const now = Date.now()
const revalidateAfter = this.calculateRevalidate(pathname, now)
data = {
value: null,
revalidateAfter: revalidateAfter !== false ? now : false,
}
}

try {
Expand Down
10 changes: 5 additions & 5 deletions packages/next/server/render.tsx
Expand Up @@ -827,11 +827,6 @@ export async function renderToHTML(
;(data as any).revalidate = false
}

// this must come after revalidate is attached
if ((renderOpts as any).isNotFound) {
return null
}

props.pageProps = Object.assign(
{},
props.pageProps,
Expand All @@ -843,6 +838,11 @@ export async function renderToHTML(
;(renderOpts as any).revalidate =
'revalidate' in data ? data.revalidate : undefined
;(renderOpts as any).pageData = props

// this must come after revalidate is added to renderOpts
if ((renderOpts as any).isNotFound) {
return null
}
}

if (getServerSideProps) {
Expand Down
1 change: 1 addition & 0 deletions test/integration/not-found-revalidate/data.txt
@@ -0,0 +1 @@
404
1 change: 1 addition & 0 deletions test/integration/not-found-revalidate/pages/404.js
Expand Up @@ -8,6 +8,7 @@ export default function Page(props) {
}

export const getStaticProps = () => {
console.log('404 getStaticProps')
return {
props: {
notFound: true,
Expand Down
@@ -0,0 +1,28 @@
import fs from 'fs'
import path from 'path'

export async function getStaticProps() {
const data = await fs.promises.readFile(
path.join(process.cwd(), 'data.txt'),
'utf8'
)

console.log('revalidate', { data })

return {
props: { data },
notFound: data.trim() === '404',
revalidate: 1,
}
}

export async function getStaticPaths() {
return {
paths: [{ params: { slug: 'first' } }],
fallback: 'blocking',
}
}

export default function Page({ data }) {
return <p id="data">{data}</p>
}
@@ -0,0 +1,21 @@
import fs from 'fs'
import path from 'path'

export async function getStaticProps() {
const data = await fs.promises.readFile(
path.join(process.cwd(), 'data.txt'),
'utf8'
)

console.log('revalidate', { data })

return {
props: { data },
notFound: data.trim() === '404',
revalidate: 1,
}
}

export default function Page({ data }) {
return <p id="data">{data}</p>
}
68 changes: 66 additions & 2 deletions test/integration/not-found-revalidate/test/index.test.js
Expand Up @@ -10,14 +10,74 @@ import {
killApp,
fetchViaHTTP,
waitFor,
check,
} from 'next-test-utils'

jest.setTimeout(1000 * 60 * 2)
const appDir = join(__dirname, '..')
const dataFile = join(appDir, 'data.txt')

let app
let appPort

const runTests = () => {
it('should revalidate page when notFund returned during build', async () => {
let res = await fetchViaHTTP(appPort, '/initial-not-found/first')
let $ = cheerio.load(await res.text())
expect(res.status).toBe(404)
expect($('#not-found').text()).toBe('404 page')

res = await fetchViaHTTP(appPort, '/initial-not-found/second')
$ = cheerio.load(await res.text())
expect(res.status).toBe(404)
expect($('#not-found').text()).toBe('404 page')

res = await fetchViaHTTP(appPort, '/initial-not-found')
$ = cheerio.load(await res.text())
expect(res.status).toBe(404)
expect($('#not-found').text()).toBe('404 page')

await fs.writeFile(dataFile, '200')

// wait for revalidation period
await waitFor(1500)
await fetchViaHTTP(appPort, '/initial-not-found/first')
await fetchViaHTTP(appPort, '/initial-not-found/second')
await fetchViaHTTP(appPort, '/initial-not-found')

// wait for revalidation to occur in background
try {
await check(async () => {
res = await fetchViaHTTP(appPort, '/initial-not-found/first')
$ = cheerio.load(await res.text())

return res.status === 200 && $('#data').text() === '200'
? 'success'
: `${res.status} - ${$('#data').text()}`
}, 'success')

await check(async () => {
res = await fetchViaHTTP(appPort, '/initial-not-found/second')
$ = cheerio.load(await res.text())

return res.status === 200 && $('#data').text() === '200'
? 'success'
: `${res.status} - ${$('#data').text()}`
}, 'success')

await check(async () => {
res = await fetchViaHTTP(appPort, '/initial-not-found')
$ = cheerio.load(await res.text())

return res.status === 200 && $('#data').text() === '200'
? 'success'
: `${res.status} - ${$('#data').text()}`
}, 'success')
} finally {
await fs.writeFile(dataFile, '404')
}
})

it('should revalidate after notFound is returned for fallback: blocking', async () => {
let res = await fetchViaHTTP(appPort, '/fallback-blocking/hello')
let $ = cheerio.load(await res.text())
Expand Down Expand Up @@ -138,9 +198,13 @@ describe('SSG notFound revalidate', () => {
describe('production mode', () => {
beforeAll(async () => {
await fs.remove(join(appDir, '.next'))
await nextBuild(appDir)
await nextBuild(appDir, undefined, {
cwd: appDir,
})
appPort = await findPort()
app = await nextStart(appDir, appPort)
app = await nextStart(appDir, appPort, {
cwd: appDir,
})
})
afterAll(() => killApp(app))

Expand Down

0 comments on commit 1697852

Please sign in to comment.