Skip to content

fix: support non-prerendered dynamic routes with fallback false #1541

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

Merged
merged 29 commits into from
Sep 9, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
61fe286
fix: bypass handler function for non-prerendered dynamic routes with …
orinokai Aug 16, 2022
b7a6872
fix: remove catch all handler redirect to support fallback false and …
orinokai Aug 16, 2022
b8e1950
feat: support custom 404 pages in all locales
orinokai Aug 16, 2022
0996adc
feat: add custom 404 handling for static routes
orinokai Aug 31, 2022
52ee02c
chore: refactor hidden path redirects
orinokai Aug 31, 2022
18c2333
feat: add support for isr 404 pages
orinokai Sep 2, 2022
8a361ae
feat: swap cache patch for forced manual revalidate
orinokai Sep 2, 2022
2073b27
chore: add docs for patches
orinokai Sep 2, 2022
046d375
feat: use revalidate header instead of server patch
orinokai Sep 6, 2022
69a4146
test: add dynamic routing and fallback tests
orinokai Sep 6, 2022
2b67d08
chore: fix eslint error
orinokai Sep 6, 2022
39d6ea9
fix: remove fallback: true handling until runtime fix lands
orinokai Sep 6, 2022
3f5e675
test: update tests while fallback:true works like fallback:blocking
orinokai Sep 6, 2022
5806400
test: fix jest tests
orinokai Sep 6, 2022
81d2d80
test: simplify cypress test names
orinokai Sep 6, 2022
bc0aace
Merge branch 'main' into rs/fallback-false-fix
orinokai Sep 6, 2022
94e732c
fix: revert header revalidation owing to side-effects
orinokai Sep 6, 2022
7a662ee
feat: patch next server to force revalidation only during cache fetches
orinokai Sep 6, 2022
dc424b3
test: update tests with new env var name
orinokai Sep 6, 2022
68461db
Merge branch 'main' into rs/fallback-false-fix
orinokai Sep 6, 2022
eec91b0
test: update test with reverted handler signature
orinokai Sep 6, 2022
f81b2c3
Merge branch 'rs/fallback-false-fix' of github.com:netlify/netlify-pl…
orinokai Sep 6, 2022
8732a49
fix: reinstate catch-all redirect for Next redirect handling
orinokai Sep 7, 2022
636d887
Merge branch 'main' into rs/fallback-false-fix
orinokai Sep 7, 2022
e056209
test: update snapshot with new redirects
orinokai Sep 7, 2022
987defd
test: update cypress tests for correct render mode
orinokai Sep 7, 2022
4f41b49
Merge branch 'rs/fallback-false-fix' of github.com:netlify/netlify-pl…
orinokai Sep 7, 2022
f77d276
Merge branch 'main' into rs/fallback-false-fix
orinokai Sep 9, 2022
55de50a
Merge branch 'main' into rs/fallback-false-fix
orinokai Sep 9, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
130 changes: 125 additions & 5 deletions cypress/integration/default/dynamic-routes.spec.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,127 @@
describe('Static Routing', () => {
it('renders correct page via SSR on a static route', () => {
cy.request('/getServerSideProps/static/').then((res) => {
expect(res.status).to.eq(200)
expect(res.headers).to.have.property('x-render-mode', 'ssr')
expect(res.body).to.contain('Sleepy Hollow')
})
})
it('serves correct static file on a static route', () => {
cy.request('/getStaticProps/static/').then((res) => {
expect(res.status).to.eq(200)
expect(res.headers).to.not.have.property('x-render-mode')
expect(res.body).to.contain('Dancing with the Stars')
})
})
it('renders correct page via ODB on a static route', () => {
cy.request('/getStaticProps/with-revalidate/').then((res) => {
expect(res.status).to.eq(200)
expect(res.headers).to.have.property('x-render-mode', 'isr')
expect(res.body).to.contain('Dancing with the Stars')
})
})
})

describe('Dynamic Routing', () => {
it('loads page', () => {
cy.visit('/shows/250')
cy.findByRole('heading').should('contain', '250')
cy.findByText('Kirby Buckets')
it('renders correct page via SSR on a dynamic route', () => {
cy.request('/getServerSideProps/1/').then((res) => {
expect(res.status).to.eq(200)
expect(res.headers).to.have.property('x-render-mode', 'ssr')
expect(res.body).to.contain('Under the Dome')
})
})
it('renders correct page via SSR on a dynamic catch-all route', () => {
cy.request('/getServerSideProps/all/1/').then((res) => {
expect(res.status).to.eq(200)
expect(res.headers).to.have.property('x-render-mode', 'ssr')
expect(res.body).to.contain('Under the Dome')
})
})
it('serves correct static file on a prerendered dynamic route with fallback: false', () => {
cy.request('/getStaticProps/1/').then((res) => {
expect(res.status).to.eq(200)
expect(res.headers).to.not.have.property('x-render-mode')
expect(res.body).to.contain('Under the Dome')
})
})
it('renders custom 404 on a non-prerendered dynamic route with fallback: false', () => {
cy.request({ url: '/getStaticProps/3/', failOnStatusCode: false }).then((res) => {
expect(res.status).to.eq(404)
expect(res.headers).to.have.property('x-render-mode', 'odb')
expect(res.body).to.contain('Custom 404')
})
})
it('serves correct static file on a prerendered dynamic route with fallback: true', () => {
cy.request('/getStaticProps/withFallback/1/').then((res) => {
expect(res.status).to.eq(200)
expect(res.headers).to.not.have.property('x-render-mode')
expect(res.body).to.contain('Under the Dome')
})
})
it('renders fallback page via ODB on a non-prerendered dynamic route with fallback: true', () => {
cy.request('/getStaticProps/withFallback/3/').then((res) => {
expect(res.status).to.eq(200)
// expect 'odb' until https://github.com/netlify/pillar-runtime/issues/438 is fixed
expect(res.headers).to.have.property('x-render-mode', 'odb')
// expect 'Bitten' until the above is fixed and we can test for fallback 'Loading...' message
expect(res.body).to.contain('Bitten')
})
})
it('serves correct static file on a prerendered dynamic route with fallback: blocking', () => {
cy.request('/getStaticProps/withFallbackBlocking/1/').then((res) => {
expect(res.status).to.eq(200)
expect(res.headers).to.not.have.property('x-render-mode')
expect(res.body).to.contain('Under the Dome')
})
})
it('renders correct page via ODB on a non-prerendered dynamic route with fallback: blocking', () => {
cy.request('/getStaticProps/withFallbackBlocking/3/').then((res) => {
expect(res.status).to.eq(200)
expect(res.headers).to.have.property('x-render-mode', 'odb')
expect(res.body).to.contain('Bitten')
})
})
it('renders correct page via ODB on a prerendered dynamic route with revalidate and fallback: false', () => {
cy.request('/getStaticProps/withRevalidate/1/').then((res) => {
expect(res.status).to.eq(200)
expect(res.headers).to.have.property('x-render-mode', 'isr')
expect(res.body).to.contain('Under the Dome')
})
})
it('renders custom 404 on a non-prerendered dynamic route with revalidate and fallback: false', () => {
cy.request({ url: '/getStaticProps/withRevalidate/3/', failOnStatusCode: false }).then((res) => {
expect(res.status).to.eq(404)
expect(res.headers).to.have.property('x-render-mode', 'odb')
expect(res.body).to.contain('Custom 404')
})
})
it('renders correct page via ODB on a prerendered dynamic route with revalidate and fallback: true', () => {
cy.request('/getStaticProps/withRevalidate/withFallback/1/').then((res) => {
expect(res.status).to.eq(200)
expect(res.headers).to.have.property('x-render-mode', 'isr')
expect(res.body).to.contain('Under the Dome')
})
})
it('renders fallback page via ODB on a non-prerendered dynamic route with revalidate and fallback: true', () => {
cy.request('/getStaticProps/withRevalidate/withFallback/3/').then((res) => {
expect(res.status).to.eq(200)
expect(res.headers).to.have.property('x-render-mode', 'isr')
// expect 'Bitten' until https://github.com/netlify/pillar-runtime/issues/438 is fixed
expect(res.body).to.contain('Bitten')
})
})
it('renders correct page via ODB on a prerendered dynamic route with revalidate and fallback: blocking', () => {
cy.request('/getStaticProps/withRevalidate/withFallbackBlocking/1/').then((res) => {
expect(res.status).to.eq(200)
expect(res.headers).to.have.property('x-render-mode', 'isr')
expect(res.body).to.contain('Under the Dome')
})
})
it('renders correct page via ODB on a non-prerendered dynamic route with revalidate and fallback: blocking', () => {
cy.request('/getStaticProps/withRevalidate/withFallbackBlocking/3/').then((res) => {
expect(res.status).to.eq(200)
expect(res.headers).to.have.property('x-render-mode', 'isr')
expect(res.body).to.contain('Bitten')
})
})
})
})
8 changes: 7 additions & 1 deletion packages/runtime/src/helpers/files.ts
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,13 @@ const getServerFile = (root: string, includeBase = true) => {
}

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

const nextServerReplacements: Array<[string, string]> = [
Expand Down
24 changes: 13 additions & 11 deletions packages/runtime/src/helpers/redirects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { RoutesManifest } from './types'
import {
getApiRewrites,
getPreviewRewrites,
is404Route,
isApiRoute,
redirectsForNextRoute,
redirectsForNextRouteWithData,
Expand All @@ -23,6 +24,14 @@ import {
const matchesMiddleware = (middleware: Array<string>, route: string): boolean =>
middleware.some((middlewarePath) => route.startsWith(middlewarePath))

const generateHiddenPathRedirects = ({ basePath }: Pick<NextConfig, 'basePath'>): NetlifyConfig['redirects'] =>
HIDDEN_PATHS.map((path) => ({
from: `${basePath}${path}`,
to: '/404.html',
status: 404,
force: true,
}))

const generateLocaleRedirects = ({
i18n,
basePath,
Expand Down Expand Up @@ -123,7 +132,7 @@ const generateStaticIsrRewrites = ({
const staticRoutePaths = new Set<string>()
const staticIsrRewrites: NetlifyConfig['redirects'] = []
staticRouteEntries.forEach(([route, { initialRevalidateSeconds }]) => {
if (isApiRoute(route)) {
if (isApiRoute(route) || is404Route(route, i18n)) {
return
}
staticRoutePaths.add(route)
Expand Down Expand Up @@ -191,7 +200,7 @@ const generateDynamicRewrites = ({
const dynamicRewrites: NetlifyConfig['redirects'] = []
const dynamicRoutesThatMatchMiddleware: Array<string> = []
dynamicRoutes.forEach((route) => {
if (isApiRoute(route.page)) {
if (isApiRoute(route.page) || is404Route(route.page, i18n)) {
return
}
if (route.page in prerenderedDynamicRoutes) {
Expand Down Expand Up @@ -231,14 +240,7 @@ export const generateRedirects = async ({
join(netlifyConfig.build.publish, 'routes-manifest.json'),
)

netlifyConfig.redirects.push(
...HIDDEN_PATHS.map((path) => ({
from: `${basePath}${path}`,
to: '/404.html',
status: 404,
force: true,
})),
)
netlifyConfig.redirects.push(...generateHiddenPathRedirects({ basePath }))

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

// Add rewrites for all static SSR routes. This is Next 12+
staticRoutes?.forEach((route) => {
if (staticRoutePaths.has(route.page) || isApiRoute(route.page)) {
if (staticRoutePaths.has(route.page) || isApiRoute(route.page) || is404Route(route.page)) {
// Prerendered static routes are either handled by the CDN or are ISR
return
}
Expand Down
3 changes: 3 additions & 0 deletions packages/runtime/src/helpers/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,9 @@ const netlifyRoutesForNextRoute = (route: string, buildId: string, i18n?: I18n):

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

export const is404Route = (route: string, i18n?: I18n) =>
i18n ? i18n.locales.some((locale) => route === `/${locale}/404`) : route === '/404'

export const redirectsForNextRoute = ({
route,
buildId,
Expand Down
2 changes: 1 addition & 1 deletion packages/runtime/src/templates/getHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ const makeHandler = (conf: NextConfig, app, pageRoot, staticManifest: Array<[str
conf.experimental.isrFlushToDisk = false
// This is our flag that we use when patching the source
// eslint-disable-next-line no-underscore-dangle
process.env._BYPASS_SSG = 'true'
process.env._REVALIDATE_SSG = 'true'
for (const [key, value] of Object.entries(conf.env)) {
process.env[key] = String(value)
}
Expand Down
36 changes: 0 additions & 36 deletions test/__snapshots__/index.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -423,12 +423,6 @@ Array [
"status": 200,
"to": "/.netlify/builders/_ipx",
},
Object {
"force": false,
"from": "/_next/data/build-id/en/404.json",
"status": 200,
"to": "/.netlify/functions/___netlify-handler",
},
Object {
"force": false,
"from": "/_next/data/build-id/en/500.json",
Expand Down Expand Up @@ -651,12 +645,6 @@ Array [
"status": 200,
"to": "/.netlify/functions/___netlify-handler",
},
Object {
"force": false,
"from": "/_next/data/build-id/es/404.json",
"status": 200,
"to": "/.netlify/functions/___netlify-handler",
},
Object {
"force": false,
"from": "/_next/data/build-id/es/500.json",
Expand Down Expand Up @@ -843,12 +831,6 @@ Array [
"status": 200,
"to": "/.netlify/functions/___netlify-handler",
},
Object {
"force": false,
"from": "/_next/data/build-id/fr/404.json",
"status": 200,
"to": "/.netlify/functions/___netlify-handler",
},
Object {
"force": false,
"from": "/_next/data/build-id/fr/500.json",
Expand Down Expand Up @@ -1072,12 +1054,6 @@ Array [
"status": 200,
"to": "/.netlify/functions/___netlify-handler",
},
Object {
"force": false,
"from": "/404",
"status": 200,
"to": "/.netlify/functions/___netlify-handler",
},
Object {
"force": false,
"from": "/500",
Expand Down Expand Up @@ -1142,12 +1118,6 @@ Array [
"status": 200,
"to": "/.netlify/functions/___netlify-handler",
},
Object {
"force": false,
"from": "/es/404",
"status": 200,
"to": "/.netlify/functions/___netlify-handler",
},
Object {
"force": false,
"from": "/es/500",
Expand Down Expand Up @@ -1340,12 +1310,6 @@ Array [
"status": 200,
"to": "/.netlify/functions/___netlify-handler",
},
Object {
"force": false,
"from": "/fr/404",
"status": 200,
"to": "/.netlify/functions/___netlify-handler",
},
Object {
"force": false,
"from": "/fr/500",
Expand Down
6 changes: 4 additions & 2 deletions test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -1030,12 +1030,14 @@ describe('utility functions', () => {
await patchNextFiles(process.cwd())
const serverFile = path.resolve(process.cwd(), 'node_modules', 'next', 'dist', 'server', 'base-server.js')
const patchedData = await readFileSync(serverFile, 'utf8')
expect(patchedData.includes('_BYPASS_SSG')).toBeTruthy()
expect(patchedData.includes('_REVALIDATE_SSG')).toBeTruthy()
expect(patchedData.includes('private: isPreviewMode && cachedData')).toBeTruthy()

await unpatchNextFiles(process.cwd())

const unPatchedData = await readFileSync(serverFile, 'utf8')
expect(unPatchedData.includes('_BYPASS_SSG')).toBeFalsy()
expect(unPatchedData.includes('_REVALIDATE_SSG')).toBeFalsy()
expect(unPatchedData.includes('private: isPreviewMode && cachedData')).toBeFalsy()
})
})

Expand Down