Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix revalidate for initial notFound: true paths #28097

Merged
merged 2 commits into from Aug 14, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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