Skip to content

Commit

Permalink
Fix static to dynamic on revalidate (#46668)
Browse files Browse the repository at this point in the history
Since it's perfectly valid to do an authorized request during revalidate we shouldn't consider this a reason to throw the static to dynamic error during runtime. If an authorized request is done during build and caching isn't enabled for a path it will still bail from being turned into a Prerender. 

x-ref: [slack thread](https://vercel.slack.com/archives/C03KAR5DCKC/p1677734108126679)

## Bug

- [x] Related issues linked using `fixes #number`
- [x] Integration tests added
- [ ] Errors have a helpful link attached, see [`contributing.md`](https://github.com/vercel/next.js/blob/canary/contributing.md)
  • Loading branch information
ijjk committed Mar 2, 2023
1 parent 6499b7b commit ed51bd8
Show file tree
Hide file tree
Showing 7 changed files with 94 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ export interface StaticGenerationStore {
readonly pathname: string
readonly incrementalCache?: import('../../server/lib/incremental-cache').IncrementalCache
readonly isRevalidate?: boolean
readonly isPrerendering?: boolean

forceDynamic?: boolean
revalidate?: false | number
Expand Down
1 change: 1 addition & 0 deletions packages/next/src/export/worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,7 @@ export default async function exportPage({
`http://localhost:3000${req.url}`
)
const staticContext: StaticGenerationContext = {
nextExport: true,
supportsDynamicHTML: false,
incrementalCache: curRenderOpts.incrementalCache,
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ export type RequestContext = {
supportsDynamicHTML: boolean
isRevalidate?: boolean
isBot?: boolean
nextExport?: boolean
}
}

Expand Down Expand Up @@ -53,6 +54,7 @@ export class StaticGenerationAsyncStorageWrapper
pathname,
incrementalCache: renderOpts.incrementalCache,
isRevalidate: renderOpts.isRevalidate,
isPrerendering: renderOpts.nextExport,
}
;(renderOpts as any).store = store

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ export type AppRouteModule = {
export type StaticGenerationContext = {
incrementalCache?: IncrementalCache
supportsDynamicHTML: boolean
nextExport?: boolean
}

/**
Expand Down
13 changes: 9 additions & 4 deletions packages/next/src/server/lib/patch-fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,12 @@ export function patchFetch({
getRequestMeta('method')?.toLowerCase() || 'get'
)

const autoNoCache = hasUnCacheableHeader || isUnCacheableMethod

if (typeof revalidate === 'undefined') {
// if there are uncacheable headers and the cache value
// wasn't overridden then we must bail static generation
if (hasUnCacheableHeader || isUnCacheableMethod) {
if (autoNoCache) {
revalidate = 0
} else {
revalidate =
Expand All @@ -93,9 +95,12 @@ export function patchFetch({
}

if (
typeof staticGenerationStore.revalidate === 'undefined' ||
(typeof revalidate === 'number' &&
revalidate < staticGenerationStore.revalidate)
// we don't consider autoNoCache to switch to dynamic during
// revalidate although if it occurs during build we do
(!autoNoCache || staticGenerationStore.isPrerendering) &&
(typeof staticGenerationStore.revalidate === 'undefined' ||
(typeof revalidate === 'number' &&
revalidate < staticGenerationStore.revalidate))
) {
staticGenerationStore.revalidate = revalidate
}
Expand Down
38 changes: 38 additions & 0 deletions test/e2e/app-dir/app-static/app-static.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ createNextDescribe(
'force-static/page.js',
'force-static/second.html',
'force-static/second.rsc',
'gen-params-dynamic-revalidate/[slug]/page.js',
'gen-params-dynamic-revalidate/one.html',
'gen-params-dynamic-revalidate/one.rsc',
'gen-params-dynamic/[slug]/page.js',
'hooks/use-pathname/[slug]/page.js',
'hooks/use-pathname/slug.html',
Expand Down Expand Up @@ -191,6 +194,11 @@ createNextDescribe(
initialRevalidateSeconds: false,
srcRoute: '/force-static/[slug]',
},
'/gen-params-dynamic-revalidate/one': {
dataRoute: '/gen-params-dynamic-revalidate/one.rsc',
initialRevalidateSeconds: 3,
srcRoute: '/gen-params-dynamic-revalidate/[slug]',
},
'/ssg-preview': {
dataRoute: '/ssg-preview.rsc',
initialRevalidateSeconds: false,
Expand Down Expand Up @@ -246,6 +254,16 @@ createNextDescribe(
fallback: null,
routeRegex: '^\\/dynamic\\-error\\/([^\\/]+?)(?:\\/)?$',
},
'/gen-params-dynamic-revalidate/[slug]': {
dataRoute: '/gen-params-dynamic-revalidate/[slug].rsc',
dataRouteRegex: normalizeRegEx(
'^\\/gen\\-params\\-dynamic\\-revalidate\\/([^\\/]+?)\\.rsc$'
),
fallback: null,
routeRegex: normalizeRegEx(
'^\\/gen\\-params\\-dynamic\\-revalidate\\/([^\\/]+?)(?:\\/)?$'
),
},
'/hooks/use-pathname/[slug]': {
dataRoute: '/hooks/use-pathname/[slug].rsc',
dataRouteRegex: normalizeRegEx(
Expand Down Expand Up @@ -793,6 +811,26 @@ createNextDescribe(
}
})

it('should not error with generateStaticParams and authed data on revalidate', async () => {
const res = await next.fetch('/gen-params-dynamic-revalidate/one')
const html = await res.text()
expect(res.status).toBe(200)
expect(html).toContain('gen-params-dynamic/[slug]')
expect(html).toContain('one')
const initData = cheerio.load(html)('#data').text()

await check(async () => {
const res2 = await next.fetch('/gen-params-dynamic-revalidate/one')

expect(res2.status).toBe(200)

const $ = cheerio.load(await res2.text())
expect($('#data').text()).toBeTruthy()
expect($('#data').text()).not.toBe(initData)
return 'success'
}, 'success')
})

it('should honor dynamic = "force-static" correctly', async () => {
const res = await next.fetch('/force-static/first')
expect(res.status).toBe(200)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
export const revalidate = 3

export async function generateStaticParams() {
return [{ slug: 'one' }]
}

const fetchRetry = async (url, init) => {
for (let i = 0; i < 5; i++) {
try {
return await fetch(url, init)
} catch (err) {
if (i === 4) {
throw err
}
console.log(`Failed to fetch`, err, `retrying...`)
}
}
}

export default async function page({ params }) {
const { slug } = params
let data

if (process.env.NEXT_PHASE !== 'phase-production-build') {
data = await fetchRetry(
'https://next-data-api-endpoint.vercel.app/api/random',
{
headers: {
Authorization: 'Bearer my-token',
},
}
).then((res) => res.text())
}

return (
<>
<p id="page">/gen-params-dynamic/[slug]</p>
<p id="slug">{slug}</p>
<p id="data">{data}</p>
</>
)
}

0 comments on commit ed51bd8

Please sign in to comment.