Skip to content

Commit

Permalink
revert bailout handling and update auto no cache only
Browse files Browse the repository at this point in the history
  • Loading branch information
@ijjk
ijjk committed May 22, 2024
1 parent 3aa79db commit 14df8a3
Show file tree
Hide file tree
Showing 20 changed files with 221 additions and 73 deletions.
15 changes: 15 additions & 0 deletions packages/next/src/server/app-render/dynamic-rendering.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,21 @@ export function Postpone({
postponeWithTracking(prerenderState, reason, pathname)
}

// @TODO refactor patch-fetch and this function to better model dynamic semantics. Currently this implementation
// is too explicit about postponing if we are in a prerender and patch-fetch contains a lot of logic for determining
// what makes the fetch "dynamic". It also doesn't handle Non PPR cases so it is isn't as consistent with the other
// dynamic-rendering methods.
export function trackDynamicFetch(
store: StaticGenerationStore,
expression: string
) {
// If we aren't in a prerender, or we're in an unstable cache callback, we
// don't need to postpone.
if (!store.prerenderState || store.isUnstableCacheCallback) return

postponeWithTracking(store.prerenderState, expression, store.urlPathname)
}

function postponeWithTracking(
prerenderState: PrerenderState,
expression: string,
Expand Down
149 changes: 116 additions & 33 deletions packages/next/src/server/lib/patch-fetch.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
NEXT_CACHE_TAG_MAX_LENGTH,
} from '../../lib/constants'
import * as Log from '../../build/output/log'
import { trackDynamicFetch } from '../app-render/dynamic-rendering'
import type { FetchMetric } from '../base-http'

const isEdgeRuntime = process.env.NEXT_RUNTIME === 'edge'
Expand Down Expand Up @@ -214,7 +215,10 @@ interface PatchableModule {

function createPatchedFetcher(
originFetch: Fetcher,
{ staticGenerationAsyncStorage }: PatchableModule
{
serverHooks: { DynamicServerError },
staticGenerationAsyncStorage,
}: PatchableModule
): PatchedFetcher {
// Create the patched fetch function. We don't set the type here, as it's
// verified as the return value of this function.
Expand Down Expand Up @@ -295,7 +299,7 @@ function createPatchedFetcher(
}
// RequestInit doesn't keep extra fields e.g. next so it's
// only available if init is used separate
let userRevalidate = getNextField('revalidate')
let currentFetchRevalidate = getNextField('revalidate')
const tags: string[] = validateTags(
getNextField('tags') || [],
`fetch ${input.toString()}`
Expand All @@ -313,49 +317,52 @@ function createPatchedFetcher(
}
const implicitTags = addImplicitTags(staticGenerationStore)

const fetchCacheMode = staticGenerationStore.fetchCache
const pageFetchCacheMode = staticGenerationStore.fetchCache
const isUsingNoStore = !!staticGenerationStore.isUnstableNoStore

let _cache = getRequestMeta('cache')
let currentFetchCacheConfig = getRequestMeta('cache')
let cacheReason = ''

if (
typeof _cache === 'string' &&
typeof userRevalidate !== 'undefined'
typeof currentFetchCacheConfig === 'string' &&
typeof currentFetchRevalidate !== 'undefined'
) {
// when providing fetch with a Request input, it'll automatically set a cache value of 'default'
// we only want to warn if the user is explicitly setting a cache value
if (!(isRequestInput && _cache === 'default')) {
if (!(isRequestInput && currentFetchCacheConfig === 'default')) {
Log.warn(
`fetch for ${fetchUrl} on ${staticGenerationStore.urlPathname} specified "cache: ${_cache}" and "revalidate: ${userRevalidate}", only one should be specified.`
`fetch for ${fetchUrl} on ${staticGenerationStore.urlPathname} specified "cache: ${currentFetchCacheConfig}" and "revalidate: ${currentFetchRevalidate}", only one should be specified.`
)
}
_cache = undefined
currentFetchCacheConfig = undefined
}

if (_cache === 'force-cache') {
userRevalidate = false
if (currentFetchCacheConfig === 'force-cache') {
currentFetchRevalidate = false
} else if (
_cache === 'no-cache' ||
_cache === 'no-store' ||
fetchCacheMode === 'force-no-store' ||
fetchCacheMode === 'only-no-store' ||
currentFetchCacheConfig === 'no-cache' ||
currentFetchCacheConfig === 'no-store' ||
pageFetchCacheMode === 'force-no-store' ||
pageFetchCacheMode === 'only-no-store' ||
// If no explicit fetch cache mode is set, but dynamic = `force-dynamic` is set,
// we shouldn't consider caching the fetch. This is because the `dynamic` cache
// is considered a "top-level" cache mode, whereas something like `fetchCache` is more
// fine-grained. Top-level modes are responsible for setting reasonable defaults for the
// other configurations.
(!fetchCacheMode && staticGenerationStore.forceDynamic)
(!pageFetchCacheMode && staticGenerationStore.forceDynamic)
) {
userRevalidate = 0
currentFetchRevalidate = 0
}

if (_cache === 'no-cache' || _cache === 'no-store') {
cacheReason = `cache: ${_cache}`
if (
currentFetchCacheConfig === 'no-cache' ||
currentFetchCacheConfig === 'no-store'
) {
cacheReason = `cache: ${currentFetchCacheConfig}`
}

finalRevalidate = validateRevalidate(
userRevalidate,
currentFetchRevalidate,
staticGenerationStore.urlPathname
)

Expand All @@ -375,18 +382,27 @@ function createPatchedFetcher(
// if there are authorized headers or a POST method and
// dynamic data usage was present above the tree we bail
// e.g. if cookies() is used before an authed/POST fetch
// or no user provided fetch cache config or revalidate
// is provided we don't cache
const autoNoCache =
(hasUnCacheableHeader || isUnCacheableMethod) &&
staticGenerationStore.revalidate === 0

switch (fetchCacheMode) {
// this condition is hit for null/undefined
// eslint-disable-next-line eqeqeq
(pageFetchCacheMode == undefined &&
// eslint-disable-next-line eqeqeq
currentFetchCacheConfig == undefined &&
// eslint-disable-next-line eqeqeq
currentFetchRevalidate == undefined) ||
((hasUnCacheableHeader || isUnCacheableMethod) &&
staticGenerationStore.revalidate === 0)

switch (pageFetchCacheMode) {
case 'force-no-store': {
cacheReason = 'fetchCache = force-no-store'
break
}
case 'only-no-store': {
if (
_cache === 'force-cache' ||
currentFetchCacheConfig === 'force-cache' ||
(typeof finalRevalidate !== 'undefined' &&
(finalRevalidate === false || finalRevalidate > 0))
) {
Expand All @@ -398,15 +414,18 @@ function createPatchedFetcher(
break
}
case 'only-cache': {
if (_cache === 'no-store') {
if (currentFetchCacheConfig === 'no-store') {
throw new Error(
`cache: 'no-store' used on fetch for ${fetchUrl} with 'export const fetchCache = 'only-cache'`
)
}
break
}
case 'force-cache': {
if (typeof userRevalidate === 'undefined' || userRevalidate === 0) {
if (
typeof currentFetchRevalidate === 'undefined' ||
currentFetchRevalidate === 0
) {
cacheReason = 'fetchCache = force-cache'
finalRevalidate = false
}
Expand All @@ -419,11 +438,14 @@ function createPatchedFetcher(
// simplify the switch case and ensure we have an exhaustive switch handling all modes
}

if (!autoNoCache && typeof finalRevalidate === 'undefined') {
if (fetchCacheMode === 'default-cache') {
if (typeof finalRevalidate === 'undefined') {
if (pageFetchCacheMode === 'default-cache') {
finalRevalidate = false
cacheReason = 'fetchCache = default-cache'
} else if (fetchCacheMode === 'default-no-store') {
} else if (autoNoCache) {
finalRevalidate = 0
cacheReason = 'auto no cache'
} else if (pageFetchCacheMode === 'default-no-store') {
finalRevalidate = 0
cacheReason = 'fetchCache = default-no-store'
} else if (isUsingNoStore) {
Expand All @@ -442,16 +464,26 @@ function createPatchedFetcher(
}

if (
finalRevalidate !== 0 &&
// when force static is configured we don't bail from
// `revalidate: 0` values
!(staticGenerationStore.forceStatic && finalRevalidate === 0) &&
// we don't consider autoNoCache to switch to dynamic for ISR
!autoNoCache &&
// If the revalidate value isn't currently set or the value is less
// than the current revalidate value, we should update the
// revalidate value.
// than the current revalidate value, we should update the revalidate
// value.
(typeof staticGenerationStore.revalidate === 'undefined' ||
(typeof finalRevalidate === 'number' &&
(staticGenerationStore.revalidate === false ||
(typeof staticGenerationStore.revalidate === 'number' &&
finalRevalidate < staticGenerationStore.revalidate))))
) {
// If we were setting the revalidate value to 0, we should try to
// postpone instead first.
if (finalRevalidate === 0) {
trackDynamicFetch(staticGenerationStore, 'revalidate: 0')
}

staticGenerationStore.revalidate = finalRevalidate
}

Expand Down Expand Up @@ -662,9 +694,35 @@ function createPatchedFetcher(
init &&
typeof init === 'object'
) {
const { cache } = init

// Delete `cache` property as Cloudflare Workers will throw an error
if (isEdgeRuntime) delete init.cache

if (!staticGenerationStore.forceStatic && cache === 'no-store') {
const dynamicUsageReason = `no-store fetch ${input}${
staticGenerationStore.urlPathname
? ` ${staticGenerationStore.urlPathname}`
: ''
}`

// If enabled, we should bail out of static generation.
trackDynamicFetch(staticGenerationStore, dynamicUsageReason)

// If partial prerendering is not enabled, then we should throw an
// error to indicate that this fetch is dynamic.
if (!staticGenerationStore.prerenderState) {
// PPR is not enabled, or React postpone is not available, we
// should set the revalidate to 0.
staticGenerationStore.revalidate = 0

const err = new DynamicServerError(dynamicUsageReason)
staticGenerationStore.dynamicUsageErr = err
staticGenerationStore.dynamicUsageDescription = dynamicUsageReason
throw err
}
}

const hasNextConfig = 'next' in init
const { next = {} } = init
if (
Expand All @@ -673,6 +731,31 @@ function createPatchedFetcher(
(typeof staticGenerationStore.revalidate === 'number' &&
next.revalidate < staticGenerationStore.revalidate))
) {
if (
!staticGenerationStore.forceDynamic &&
!staticGenerationStore.forceStatic &&
next.revalidate === 0
) {
const dynamicUsageReason = `revalidate: 0 fetch ${input}${
staticGenerationStore.urlPathname
? ` ${staticGenerationStore.urlPathname}`
: ''
}`

// If enabled, we should bail out of static generation.
trackDynamicFetch(staticGenerationStore, dynamicUsageReason)

// If partial prerendering is not enabled, then we should throw an
// error to indicate that this fetch is dynamic.
if (!staticGenerationStore.prerenderState) {
const err = new DynamicServerError(dynamicUsageReason)
staticGenerationStore.dynamicUsageErr = err
staticGenerationStore.dynamicUsageDescription =
dynamicUsageReason
throw err
}
}

if (!staticGenerationStore.forceStatic || next.revalidate !== 0) {
staticGenerationStore.revalidate = next.revalidate
}
Expand Down
60 changes: 60 additions & 0 deletions test/e2e/app-dir/app-static/app-static.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,33 @@ describe('app-dir static/dynamic handling', () => {
}
})

it('should use auto no cache when no fetch config', async () => {
const res = await next.fetch('/no-config-fetch')
expect(res.status).toBe(200)

const html = await res.text()
const $ = cheerio.load(html)
const data = $('#data').text()

expect(data).toBeTruthy()

const res2 = await next.fetch('/no-config-fetch')
const html2 = await res2.text()
const data2 = cheerio.load(html2)('#data').text()

if (isNextDev) {
expect(data).not.toBe(data2)
} else {
const pageCache = (
res.headers.get('x-vercel-cache') || res.headers.get('x-nextjs-cache')
).toLowerCase()

expect(pageCache).toBeTruthy()
expect(pageCache).not.toBe('MISS')
expect(data).toBe(data2)
}
})

it('should still cache even though the `traceparent` header was different', async () => {
const res = await next.fetch('/strip-header-traceparent')
expect(res.status).toBe(200)
Expand Down Expand Up @@ -642,6 +669,8 @@ describe('app-dir static/dynamic handling', () => {
[
"(new)/custom/page.js",
"(new)/custom/page_client-reference-manifest.js",
"(new)/no-config-fetch/page.js",
"(new)/no-config-fetch/page_client-reference-manifest.js",
"_not-found.html",
"_not-found.rsc",
"_not-found/page.js",
Expand Down Expand Up @@ -704,6 +733,8 @@ describe('app-dir static/dynamic handling', () => {
"force-dynamic-no-prerender/[id]/page_client-reference-manifest.js",
"force-dynamic-prerender/[slug]/page.js",
"force-dynamic-prerender/[slug]/page_client-reference-manifest.js",
"force-no-store-bailout/page.js",
"force-no-store-bailout/page_client-reference-manifest.js",
"force-no-store/page.js",
"force-no-store/page_client-reference-manifest.js",
"force-static-fetch-no-store.html",
Expand Down Expand Up @@ -742,6 +773,8 @@ describe('app-dir static/dynamic handling', () => {
"isr-error-handling.rsc",
"isr-error-handling/page.js",
"isr-error-handling/page_client-reference-manifest.js",
"no-config-fetch.html",
"no-config-fetch.rsc",
"no-store/dynamic/page.js",
"no-store/dynamic/page_client-reference-manifest.js",
"no-store/static.html",
Expand Down Expand Up @@ -1231,6 +1264,22 @@ describe('app-dir static/dynamic handling', () => {
"initialRevalidateSeconds": 3,
"srcRoute": "/isr-error-handling",
},
"/no-config-fetch": {
"dataRoute": "/no-config-fetch.rsc",
"experimentalBypassFor": [
{
"key": "Next-Action",
"type": "header",
},
{
"key": "content-type",
"type": "header",
"value": "multipart/form-data;.*",
},
],
"initialRevalidateSeconds": false,
"srcRoute": "/no-config-fetch",
},
"/no-store/static": {
"dataRoute": "/no-store/static.rsc",
"experimentalBypassFor": [
Expand Down Expand Up @@ -1902,6 +1951,17 @@ describe('app-dir static/dynamic handling', () => {
`)
})

it('should output debug info for static bailouts', async () => {
const cleanedOutput = stripAnsi(next.cliOutput)

expect(cleanedOutput).toContain(
'Static generation failed due to dynamic usage on /force-static, reason: headers'
)
expect(cleanedOutput).toContain(
'Static generation failed due to dynamic usage on /ssr-auto/cache-no-store, reason: no-store fetch'
)
})

// build cache not leveraged for custom cache handler so not seeded
if (!process.env.CUSTOM_CACHE_HANDLER) {
it('should correctly error and not update cache for ISR', async () => {
Expand Down
2 changes: 0 additions & 2 deletions test/e2e/app-dir/app-static/app/(new)/layout.js
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
export const fetchCache = 'default-cache'

export default function Layout({ children }) {
return (
<html lang="en">
Expand Down
Loading

0 comments on commit 14df8a3

Please sign in to comment.