Skip to content

Commit 80db8f4

Browse files
piehpieh
authored
fix: handle cacheComponents case not producing prerendered .rsc files (#3275)
1 parent 071c9a0 commit 80db8f4

File tree

5 files changed

+95
-58
lines changed

5 files changed

+95
-58
lines changed

‎src/build/content/prerendered.ts‎

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ const RSC_SEGMENT_SUFFIX = '.segment.rsc'
7676
const buildAppCacheValue = async (
7777
path: string,
7878
shouldUseAppPageKind: boolean,
79+
rscIsRequired = true,
7980
): Promise<NetlifyCachedAppPageValue | NetlifyCachedPageValue> => {
8081
const meta = JSON.parse(await readFile(`${path}.meta`, 'utf-8')) as RouteMetadata
8182
const html = await readFile(`${path}.html`, 'utf-8')
@@ -104,9 +105,16 @@ const buildAppCacheValue = async (
104105
return {
105106
kind: 'APP_PAGE',
106107
html,
107-
rscData: await readFile(`${path}.rsc`, 'base64').catch(() =>
108-
readFile(`${path}.prefetch.rsc`, 'base64'),
109-
),
108+
rscData: await readFile(`${path}.rsc`, 'base64')
109+
.catch(() => readFile(`${path}.prefetch.rsc`, 'base64'))
110+
.catch((error) => {
111+
if (rscIsRequired) {
112+
throw error
113+
}
114+
// disabling unicorn/no-useless-undefined because we need to return undefined explicitly to satisfy types
115+
// eslint-disable-next-line unicorn/no-useless-undefined
116+
return undefined
117+
}),
110118
segmentData,
111119
...meta,
112120
}
@@ -186,6 +194,8 @@ export const copyPrerenderedContent = async (ctx: PluginContext): Promise<void>
186194
})
187195
: false
188196

197+
let appRouterNotFoundDefinedInPrerenderManifest = false
198+
189199
await Promise.all([
190200
...Object.entries(manifest.routes).map(
191201
([route, meta]): Promise<void> =>
@@ -215,7 +225,11 @@ export const copyPrerenderedContent = async (ctx: PluginContext): Promise<void>
215225
value = await buildAppCacheValue(
216226
join(ctx.publishDir, 'server/app', key),
217227
shouldUseAppPageKind,
228+
meta.renderingMode !== 'PARTIALLY_STATIC',
218229
)
230+
if (route === '/_not-found') {
231+
appRouterNotFoundDefinedInPrerenderManifest = true
232+
}
219233
break
220234
case meta.dataRoute === null:
221235
value = await buildRouteCacheValue(
@@ -262,9 +276,12 @@ export const copyPrerenderedContent = async (ctx: PluginContext): Promise<void>
262276
),
263277
])
264278

265-
// app router 404 pages are not in the prerender manifest
266-
// so we need to check for them manually
267-
if (existsSync(join(ctx.publishDir, `server/app/_not-found.html`))) {
279+
// app router 404 pages are not in the prerender manifest for some next.js versions
280+
// so we need to check for them manually if prerender manifest does not include it
281+
if (
282+
!appRouterNotFoundDefinedInPrerenderManifest &&
283+
existsSync(join(ctx.publishDir, `server/app/_not-found.html`))
284+
) {
268285
const lastModified = Date.now()
269286
const key = '/404'
270287
const value = await buildAppCacheValue(

‎tests/fixtures/ppr/package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
},
1616
"test": {
1717
"dependencies": {
18-
"next": "canary"
18+
"next": "canary || >=16.0.0"
1919
}
2020
}
2121
}

‎tests/integration/cache-handler.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,7 @@ describe('app router', () => {
218218
const blobEntries = await getBlobEntries(ctx)
219219
expect(blobEntries.map(({ key }) => decodeBlobKey(key)).sort()).toEqual(
220220
[
221-
'/404',
221+
shouldHaveAppRouterNotFoundInPrerenderManifest() ? undefined : '/404',
222222
shouldHaveAppRouterGlobalErrorInPrerenderManifest() ? '/_global-error' : undefined,
223223
shouldHaveAppRouterNotFoundInPrerenderManifest() ? '/_not-found' : undefined,
224224
'/index',
@@ -372,7 +372,7 @@ describe('plugin', () => {
372372
const blobEntries = await getBlobEntries(ctx)
373373
expect(blobEntries.map(({ key }) => decodeBlobKey(key)).sort()).toEqual(
374374
[
375-
'/404',
375+
shouldHaveAppRouterNotFoundInPrerenderManifest() ? undefined : '/404',
376376
shouldHaveAppRouterGlobalErrorInPrerenderManifest() ? '/_global-error' : undefined,
377377
shouldHaveAppRouterNotFoundInPrerenderManifest() ? '/_not-found' : undefined,
378378
'/api/revalidate-handler',

‎tests/integration/simple-app.test.ts‎

Lines changed: 32 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ test<FixtureTestContext>('Test that the simple next app is working', async (ctx)
104104
const blobEntries = await getBlobEntries(ctx)
105105
expect(blobEntries.map(({ key }) => decodeBlobKey(key)).sort()).toEqual(
106106
[
107-
'/404',
107+
shouldHaveAppRouterNotFoundInPrerenderManifest() ? undefined : '/404',
108108
shouldHaveAppRouterNotFoundInPrerenderManifest() ? '/_not-found' : undefined,
109109
'/api/cached-permanent',
110110
'/api/cached-revalidate',
@@ -392,41 +392,40 @@ test<FixtureTestContext>('rewrites to external addresses dont use compression',
392392
expect(gunzipSync(page.bodyBuffer).toString('utf-8')).toContain('<title>Example Domain</title>')
393393
})
394394

395-
test.skipIf(process.env.NEXT_VERSION !== 'canary')<FixtureTestContext>(
396-
'Test that a simple next app with PPR is working',
397-
async (ctx) => {
398-
await createFixture('ppr', ctx)
399-
await runPlugin(ctx)
400-
// check if the blob entries where successful set on the build plugin
401-
const blobEntries = await getBlobEntries(ctx)
402-
expect(blobEntries.map(({ key }) => decodeBlobKey(key)).sort()).toEqual(
403-
[
404-
'/1',
405-
'/2',
406-
'/404',
407-
isExperimentalPPRHardDeprecated() ? undefined : '/[dynamic]',
408-
shouldHaveAppRouterGlobalErrorInPrerenderManifest() ? '/_global-error' : undefined,
409-
shouldHaveAppRouterNotFoundInPrerenderManifest() ? '/_not-found' : undefined,
410-
'/index',
411-
'404.html',
412-
'500.html',
413-
].filter(Boolean),
414-
)
395+
test.skipIf(
396+
process.env.NEXT_VERSION !== 'canary' && nextVersionSatisfies('<16.0.0'),
397+
)<FixtureTestContext>('Test that a simple next app with PPR is working', async (ctx) => {
398+
await createFixture('ppr', ctx)
399+
await runPlugin(ctx)
400+
// check if the blob entries where successful set on the build plugin
401+
const blobEntries = await getBlobEntries(ctx)
402+
expect(blobEntries.map(({ key }) => decodeBlobKey(key)).sort()).toEqual(
403+
[
404+
'/1',
405+
'/2',
406+
shouldHaveAppRouterNotFoundInPrerenderManifest() ? undefined : '/404',
407+
isExperimentalPPRHardDeprecated() ? undefined : '/[dynamic]',
408+
shouldHaveAppRouterGlobalErrorInPrerenderManifest() ? '/_global-error' : undefined,
409+
shouldHaveAppRouterNotFoundInPrerenderManifest() ? '/_not-found' : undefined,
410+
'/index',
411+
'404.html',
412+
'500.html',
413+
].filter(Boolean),
414+
)
415415

416-
// test the function call
417-
const home = await invokeFunction(ctx)
418-
expect(home.statusCode).toBe(200)
419-
expect(load(home.body)('h1').text()).toBe('Home')
416+
// test the function call
417+
const home = await invokeFunction(ctx)
418+
expect(home.statusCode).toBe(200)
419+
expect(load(home.body)('h1').text()).toBe('Home')
420420

421-
const dynamicPrerendered = await invokeFunction(ctx, { url: '/1' })
422-
expect(dynamicPrerendered.statusCode).toBe(200)
423-
expect(load(dynamicPrerendered.body)('h1').text()).toBe('Dynamic Page: 1')
421+
const dynamicPrerendered = await invokeFunction(ctx, { url: '/1' })
422+
expect(dynamicPrerendered.statusCode).toBe(200)
423+
expect(load(dynamicPrerendered.body)('h1').text()).toBe('Dynamic Page: 1')
424424

425-
const dynamicNotPrerendered = await invokeFunction(ctx, { url: '/3' })
426-
expect(dynamicNotPrerendered.statusCode).toBe(200)
427-
expect(load(dynamicNotPrerendered.body)('h1').text()).toBe('Dynamic Page: 3')
428-
},
429-
)
425+
const dynamicNotPrerendered = await invokeFunction(ctx, { url: '/3' })
426+
expect(dynamicNotPrerendered.statusCode).toBe(200)
427+
expect(load(dynamicNotPrerendered.body)('h1').text()).toBe('Dynamic Page: 3')
428+
})
430429

431430
// setup for this test only works with webpack builds due to usage of ` __non_webpack_require__` to avoid bundling a file
432431
test.skipIf(hasDefaultTurbopackBuilds())<FixtureTestContext>(

‎tests/utils/next-version-helpers.mjs‎

Lines changed: 37 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import fg from 'fast-glob'
66
import { coerce, gt, gte, satisfies, valid } from 'semver'
77
import { execaCommand } from 'execa'
88

9-
const FUTURE_NEXT_PATCH_VERSION = '15.999.0'
9+
const FUTURE_NEXT_PATCH_VERSION = '16.999.0'
1010

1111
const NEXT_VERSION_REQUIRES_REACT_19 = '14.3.0-canary.45'
1212
const REACT_18_VERSION = '18.2.0'
@@ -114,23 +114,44 @@ export async function setNextVersionInFixture(
114114
packageJsons.map(async (packageJsonPath) => {
115115
const packageJson = JSON.parse(await readFile(packageJsonPath, 'utf8'))
116116
if (packageJson.dependencies?.next) {
117-
const versionConstraint = packageJson.test?.dependencies?.next
118-
// We can't use semver to check "canary" or "latest", so we use a fake future minor version
119-
const checkVersion = isSemverVersion ? resolvedVersion : FUTURE_NEXT_PATCH_VERSION
120-
if (
121-
operation === 'update' &&
122-
versionConstraint &&
123-
!(versionConstraint === 'canary'
124-
? isNextCanary()
125-
: satisfies(checkVersion, versionConstraint, { includePrerelease: true })) &&
126-
version !== versionConstraint
127-
) {
128-
if (!silent) {
129-
console.log(
130-
`${logPrefix}⏩ Skipping '${packageJson.name}' because it requires next@${versionConstraint}`,
117+
/** @type {string | undefined} */
118+
const versionConstraints = packageJson.test?.dependencies?.next
119+
120+
if (versionConstraints) {
121+
// We need to be able to define constraint such as "canary or >=16.0.0" for testing features that might have
122+
// canary-only support (you can only even try them on canary releases) that later on get promoted to be usable in stable
123+
// releases.
124+
// There is no proper semver range to express "canary" portion of it, so we have to implement custom logic here
125+
126+
if (versionConstraints.includes('(') || versionConstraints.includes(')')) {
127+
// We currently don't have a use case for complex semver ranges, so to simplify the implementation we literally
128+
// just split on '||' and check each part separately. This might not work work with '()' groups in semver ranges
129+
// so we throw here for clarity that it's not supported
130+
throw new Error(
131+
`${logPrefix}Complex semver ranges with '()' groups are not supported in test.dependencies.next: ${versionConstraints}`,
131132
)
132133
}
133-
return { packageJsonPath, needUpdate: false }
134+
135+
// We can't use semver to check "canary" or "latest", so we use a fake future minor version
136+
const checkVersion = isSemverVersion ? resolvedVersion : FUTURE_NEXT_PATCH_VERSION
137+
138+
if (
139+
operation === 'update' &&
140+
version !== versionConstraints &&
141+
!versionConstraints.split('||').some((versionConstraintUntrimmedPart) => {
142+
const versionConstraintPart = versionConstraintUntrimmedPart.trim()
143+
return versionConstraintPart === 'canary'
144+
? isNextCanary()
145+
: satisfies(checkVersion, versionConstraintPart, { includePrerelease: true })
146+
})
147+
) {
148+
if (!silent) {
149+
console.log(
150+
`${logPrefix}⏩ Skipping '${packageJson.name}' because it requires next@${versionConstraints}`,
151+
)
152+
}
153+
return { packageJsonPath, needUpdate: false }
154+
}
134155
}
135156
}
136157
return { packageJsonPath, needUpdate: true }

0 commit comments

Comments
 (0)