Skip to content

Commit 92a015c

Browse files
orinokaiorinokai
authored
fix: support non-prerendered dynamic routes with fallback false (#1541)
* fix: bypass handler function for non-prerendered dynamic routes with fallback: false * fix: remove catch all handler redirect to support fallback false and custom 404s * feat: support custom 404 pages in all locales * feat: add custom 404 handling for static routes * chore: refactor hidden path redirects * feat: add support for isr 404 pages * feat: swap cache patch for forced manual revalidate * chore: add docs for patches * feat: use revalidate header instead of server patch * test: add dynamic routing and fallback tests * chore: fix eslint error * fix: remove fallback: true handling until runtime fix lands * test: update tests while fallback:true works like fallback:blocking * test: fix jest tests * test: simplify cypress test names * fix: revert header revalidation owing to side-effects * feat: patch next server to force revalidation only during cache fetches * test: update tests with new env var name * test: update test with reverted handler signature * fix: reinstate catch-all redirect for Next redirect handling * test: update snapshot with new redirects * test: update cypress tests for correct render mode
1 parent fd88b98 commit 92a015c

File tree

7 files changed

+153
-56
lines changed

7 files changed

+153
-56
lines changed

‎cypress/integration/default/dynamic-routes.spec.ts

+125-5
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,127 @@
1+
describe('Static Routing', () => {
2+
it('renders correct page via SSR on a static route', () => {
3+
cy.request('/getServerSideProps/static/').then((res) => {
4+
expect(res.status).to.eq(200)
5+
expect(res.headers).to.have.property('x-render-mode', 'ssr')
6+
expect(res.body).to.contain('Sleepy Hollow')
7+
})
8+
})
9+
it('serves correct static file on a static route', () => {
10+
cy.request('/getStaticProps/static/').then((res) => {
11+
expect(res.status).to.eq(200)
12+
expect(res.headers).to.not.have.property('x-render-mode')
13+
expect(res.body).to.contain('Dancing with the Stars')
14+
})
15+
})
16+
it('renders correct page via ODB on a static route', () => {
17+
cy.request('/getStaticProps/with-revalidate/').then((res) => {
18+
expect(res.status).to.eq(200)
19+
expect(res.headers).to.have.property('x-render-mode', 'isr')
20+
expect(res.body).to.contain('Dancing with the Stars')
21+
})
22+
})
23+
})
24+
125
describe('Dynamic Routing', () => {
2-
it('loads page', () => {
3-
cy.visit('/shows/250')
4-
cy.findByRole('heading').should('contain', '250')
5-
cy.findByText('Kirby Buckets')
26+
it('renders correct page via SSR on a dynamic route', () => {
27+
cy.request('/getServerSideProps/1/').then((res) => {
28+
expect(res.status).to.eq(200)
29+
expect(res.headers).to.have.property('x-render-mode', 'ssr')
30+
expect(res.body).to.contain('Under the Dome')
31+
})
32+
})
33+
it('renders correct page via SSR on a dynamic catch-all route', () => {
34+
cy.request('/getServerSideProps/all/1/').then((res) => {
35+
expect(res.status).to.eq(200)
36+
expect(res.headers).to.have.property('x-render-mode', 'ssr')
37+
expect(res.body).to.contain('Under the Dome')
38+
})
39+
})
40+
it('serves correct static file on a prerendered dynamic route with fallback: false', () => {
41+
cy.request('/getStaticProps/1/').then((res) => {
42+
expect(res.status).to.eq(200)
43+
expect(res.headers).to.not.have.property('x-render-mode')
44+
expect(res.body).to.contain('Under the Dome')
45+
})
46+
})
47+
it('renders custom 404 on a non-prerendered dynamic route with fallback: false', () => {
48+
cy.request({ url: '/getStaticProps/3/', failOnStatusCode: false }).then((res) => {
49+
expect(res.status).to.eq(404)
50+
expect(res.headers).to.have.property('x-render-mode', 'odb')
51+
expect(res.body).to.contain('Custom 404')
52+
})
53+
})
54+
it('serves correct static file on a prerendered dynamic route with fallback: true', () => {
55+
cy.request('/getStaticProps/withFallback/1/').then((res) => {
56+
expect(res.status).to.eq(200)
57+
expect(res.headers).to.not.have.property('x-render-mode')
58+
expect(res.body).to.contain('Under the Dome')
59+
})
60+
})
61+
it('renders fallback page via ODB on a non-prerendered dynamic route with fallback: true', () => {
62+
cy.request('/getStaticProps/withFallback/3/').then((res) => {
63+
expect(res.status).to.eq(200)
64+
// expect 'odb' until https://github.com/netlify/pillar-runtime/issues/438 is fixed
65+
expect(res.headers).to.have.property('x-render-mode', 'odb')
66+
// expect 'Bitten' until the above is fixed and we can test for fallback 'Loading...' message
67+
expect(res.body).to.contain('Bitten')
68+
})
69+
})
70+
it('serves correct static file on a prerendered dynamic route with fallback: blocking', () => {
71+
cy.request('/getStaticProps/withFallbackBlocking/1/').then((res) => {
72+
expect(res.status).to.eq(200)
73+
expect(res.headers).to.not.have.property('x-render-mode')
74+
expect(res.body).to.contain('Under the Dome')
75+
})
76+
})
77+
it('renders correct page via ODB on a non-prerendered dynamic route with fallback: blocking', () => {
78+
cy.request('/getStaticProps/withFallbackBlocking/3/').then((res) => {
79+
expect(res.status).to.eq(200)
80+
expect(res.headers).to.have.property('x-render-mode', 'odb')
81+
expect(res.body).to.contain('Bitten')
82+
})
83+
})
84+
it('renders correct page via ODB on a prerendered dynamic route with revalidate and fallback: false', () => {
85+
cy.request('/getStaticProps/withRevalidate/1/').then((res) => {
86+
expect(res.status).to.eq(200)
87+
expect(res.headers).to.have.property('x-render-mode', 'isr')
88+
expect(res.body).to.contain('Under the Dome')
89+
})
90+
})
91+
it('renders custom 404 on a non-prerendered dynamic route with revalidate and fallback: false', () => {
92+
cy.request({ url: '/getStaticProps/withRevalidate/3/', failOnStatusCode: false }).then((res) => {
93+
expect(res.status).to.eq(404)
94+
expect(res.headers).to.have.property('x-render-mode', 'odb')
95+
expect(res.body).to.contain('Custom 404')
96+
})
97+
})
98+
it('renders correct page via ODB on a prerendered dynamic route with revalidate and fallback: true', () => {
99+
cy.request('/getStaticProps/withRevalidate/withFallback/1/').then((res) => {
100+
expect(res.status).to.eq(200)
101+
expect(res.headers).to.have.property('x-render-mode', 'isr')
102+
expect(res.body).to.contain('Under the Dome')
103+
})
104+
})
105+
it('renders fallback page via ODB on a non-prerendered dynamic route with revalidate and fallback: true', () => {
106+
cy.request('/getStaticProps/withRevalidate/withFallback/3/').then((res) => {
107+
expect(res.status).to.eq(200)
108+
expect(res.headers).to.have.property('x-render-mode', 'isr')
109+
// expect 'Bitten' until https://github.com/netlify/pillar-runtime/issues/438 is fixed
110+
expect(res.body).to.contain('Bitten')
111+
})
112+
})
113+
it('renders correct page via ODB on a prerendered dynamic route with revalidate and fallback: blocking', () => {
114+
cy.request('/getStaticProps/withRevalidate/withFallbackBlocking/1/').then((res) => {
115+
expect(res.status).to.eq(200)
116+
expect(res.headers).to.have.property('x-render-mode', 'isr')
117+
expect(res.body).to.contain('Under the Dome')
118+
})
119+
})
120+
it('renders correct page via ODB on a non-prerendered dynamic route with revalidate and fallback: blocking', () => {
121+
cy.request('/getStaticProps/withRevalidate/withFallbackBlocking/3/').then((res) => {
122+
expect(res.status).to.eq(200)
123+
expect(res.headers).to.have.property('x-render-mode', 'isr')
124+
expect(res.body).to.contain('Bitten')
125+
})
6126
})
7-
})
127+
})

‎packages/runtime/src/helpers/files.ts

+7-1
Original file line numberDiff line numberDiff line change
@@ -334,7 +334,13 @@ const getServerFile = (root: string, includeBase = true) => {
334334
}
335335

336336
const baseServerReplacements: Array<[string, string]> = [
337-
[`let ssgCacheKey = `, `let ssgCacheKey = process.env._BYPASS_SSG || `],
337+
// force manual revalidate during cache fetches
338+
[
339+
`checkIsManualRevalidate(req, this.renderOpts.previewProps)`,
340+
`checkIsManualRevalidate(process.env._REVALIDATE_SSG ? { headers: { 'x-prerender-revalidate': this.renderOpts.previewProps.previewModeId } } : req, this.renderOpts.previewProps)`,
341+
],
342+
// ensure ISR 404 pages send the correct SWR cache headers
343+
[`private: isPreviewMode || is404Page && cachedData`, `private: isPreviewMode && cachedData`],
338344
]
339345

340346
const nextServerReplacements: Array<[string, string]> = [

‎packages/runtime/src/helpers/redirects.ts

+13-11
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import { RoutesManifest } from './types'
1414
import {
1515
getApiRewrites,
1616
getPreviewRewrites,
17+
is404Route,
1718
isApiRoute,
1819
redirectsForNextRoute,
1920
redirectsForNextRouteWithData,
@@ -23,6 +24,14 @@ import {
2324
const matchesMiddleware = (middleware: Array<string>, route: string): boolean =>
2425
middleware.some((middlewarePath) => route.startsWith(middlewarePath))
2526

27+
const generateHiddenPathRedirects = ({ basePath }: Pick<NextConfig, 'basePath'>): NetlifyConfig['redirects'] =>
28+
HIDDEN_PATHS.map((path) => ({
29+
from: `${basePath}${path}`,
30+
to: '/404.html',
31+
status: 404,
32+
force: true,
33+
}))
34+
2635
const generateLocaleRedirects = ({
2736
i18n,
2837
basePath,
@@ -123,7 +132,7 @@ const generateStaticIsrRewrites = ({
123132
const staticRoutePaths = new Set<string>()
124133
const staticIsrRewrites: NetlifyConfig['redirects'] = []
125134
staticRouteEntries.forEach(([route, { initialRevalidateSeconds }]) => {
126-
if (isApiRoute(route)) {
135+
if (isApiRoute(route) || is404Route(route, i18n)) {
127136
return
128137
}
129138
staticRoutePaths.add(route)
@@ -191,7 +200,7 @@ const generateDynamicRewrites = ({
191200
const dynamicRewrites: NetlifyConfig['redirects'] = []
192201
const dynamicRoutesThatMatchMiddleware: Array<string> = []
193202
dynamicRoutes.forEach((route) => {
194-
if (isApiRoute(route.page)) {
203+
if (isApiRoute(route.page) || is404Route(route.page, i18n)) {
195204
return
196205
}
197206
if (route.page in prerenderedDynamicRoutes) {
@@ -231,14 +240,7 @@ export const generateRedirects = async ({
231240
join(netlifyConfig.build.publish, 'routes-manifest.json'),
232241
)
233242

234-
netlifyConfig.redirects.push(
235-
...HIDDEN_PATHS.map((path) => ({
236-
from: `${basePath}${path}`,
237-
to: '/404.html',
238-
status: 404,
239-
force: true,
240-
})),
241-
)
243+
netlifyConfig.redirects.push(...generateHiddenPathRedirects({ basePath }))
242244

243245
if (i18n && i18n.localeDetection !== false) {
244246
netlifyConfig.redirects.push(...generateLocaleRedirects({ i18n, basePath, trailingSlash }))
@@ -274,7 +276,7 @@ export const generateRedirects = async ({
274276

275277
// Add rewrites for all static SSR routes. This is Next 12+
276278
staticRoutes?.forEach((route) => {
277-
if (staticRoutePaths.has(route.page) || isApiRoute(route.page)) {
279+
if (staticRoutePaths.has(route.page) || isApiRoute(route.page) || is404Route(route.page)) {
278280
// Prerendered static routes are either handled by the CDN or are ISR
279281
return
280282
}

‎packages/runtime/src/helpers/utils.ts

+3
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,9 @@ const netlifyRoutesForNextRoute = (route: string, buildId: string, i18n?: I18n):
7979

8080
export const isApiRoute = (route: string) => route.startsWith('/api/') || route === '/api'
8181

82+
export const is404Route = (route: string, i18n?: I18n) =>
83+
i18n ? i18n.locales.some((locale) => route === `/${locale}/404`) : route === '/404'
84+
8285
export const redirectsForNextRoute = ({
8386
route,
8487
buildId,

‎packages/runtime/src/templates/getHandler.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ const makeHandler = (conf: NextConfig, app, pageRoot, staticManifest: Array<[str
4949
conf.experimental.isrFlushToDisk = false
5050
// This is our flag that we use when patching the source
5151
// eslint-disable-next-line no-underscore-dangle
52-
process.env._BYPASS_SSG = 'true'
52+
process.env._REVALIDATE_SSG = 'true'
5353
for (const [key, value] of Object.entries(conf.env)) {
5454
process.env[key] = String(value)
5555
}

‎test/__snapshots__/index.js.snap

-36
Original file line numberDiff line numberDiff line change
@@ -423,12 +423,6 @@ Array [
423423
"status": 200,
424424
"to": "/.netlify/builders/_ipx",
425425
},
426-
Object {
427-
"force": false,
428-
"from": "/_next/data/build-id/en/404.json",
429-
"status": 200,
430-
"to": "/.netlify/functions/___netlify-handler",
431-
},
432426
Object {
433427
"force": false,
434428
"from": "/_next/data/build-id/en/500.json",
@@ -651,12 +645,6 @@ Array [
651645
"status": 200,
652646
"to": "/.netlify/functions/___netlify-handler",
653647
},
654-
Object {
655-
"force": false,
656-
"from": "/_next/data/build-id/es/404.json",
657-
"status": 200,
658-
"to": "/.netlify/functions/___netlify-handler",
659-
},
660648
Object {
661649
"force": false,
662650
"from": "/_next/data/build-id/es/500.json",
@@ -843,12 +831,6 @@ Array [
843831
"status": 200,
844832
"to": "/.netlify/functions/___netlify-handler",
845833
},
846-
Object {
847-
"force": false,
848-
"from": "/_next/data/build-id/fr/404.json",
849-
"status": 200,
850-
"to": "/.netlify/functions/___netlify-handler",
851-
},
852834
Object {
853835
"force": false,
854836
"from": "/_next/data/build-id/fr/500.json",
@@ -1072,12 +1054,6 @@ Array [
10721054
"status": 200,
10731055
"to": "/.netlify/functions/___netlify-handler",
10741056
},
1075-
Object {
1076-
"force": false,
1077-
"from": "/404",
1078-
"status": 200,
1079-
"to": "/.netlify/functions/___netlify-handler",
1080-
},
10811057
Object {
10821058
"force": false,
10831059
"from": "/500",
@@ -1142,12 +1118,6 @@ Array [
11421118
"status": 200,
11431119
"to": "/.netlify/functions/___netlify-handler",
11441120
},
1145-
Object {
1146-
"force": false,
1147-
"from": "/es/404",
1148-
"status": 200,
1149-
"to": "/.netlify/functions/___netlify-handler",
1150-
},
11511121
Object {
11521122
"force": false,
11531123
"from": "/es/500",
@@ -1340,12 +1310,6 @@ Array [
13401310
"status": 200,
13411311
"to": "/.netlify/functions/___netlify-handler",
13421312
},
1343-
Object {
1344-
"force": false,
1345-
"from": "/fr/404",
1346-
"status": 200,
1347-
"to": "/.netlify/functions/___netlify-handler",
1348-
},
13491313
Object {
13501314
"force": false,
13511315
"from": "/fr/500",

‎test/index.js

+4-2
Original file line numberDiff line numberDiff line change
@@ -1030,12 +1030,14 @@ describe('utility functions', () => {
10301030
await patchNextFiles(process.cwd())
10311031
const serverFile = path.resolve(process.cwd(), 'node_modules', 'next', 'dist', 'server', 'base-server.js')
10321032
const patchedData = await readFileSync(serverFile, 'utf8')
1033-
expect(patchedData.includes('_BYPASS_SSG')).toBeTruthy()
1033+
expect(patchedData.includes('_REVALIDATE_SSG')).toBeTruthy()
1034+
expect(patchedData.includes('private: isPreviewMode && cachedData')).toBeTruthy()
10341035

10351036
await unpatchNextFiles(process.cwd())
10361037

10371038
const unPatchedData = await readFileSync(serverFile, 'utf8')
1038-
expect(unPatchedData.includes('_BYPASS_SSG')).toBeFalsy()
1039+
expect(unPatchedData.includes('_REVALIDATE_SSG')).toBeFalsy()
1040+
expect(unPatchedData.includes('private: isPreviewMode && cachedData')).toBeFalsy()
10391041
})
10401042
})
10411043

0 commit comments

Comments
 (0)