Skip to content
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

Add fetching 404 SSG data on fallback notFound #18214

Merged
merged 2 commits into from Oct 27, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
36 changes: 22 additions & 14 deletions packages/next/next-server/lib/router/router.ts
Expand Up @@ -936,13 +936,23 @@ export default class Router implements BaseRouter {
try {
let Component: ComponentType
let styleSheets: StyleSheetTuple[]
let props: Record<string, any> | undefined
const ssg404 = err.message === SSG_DATA_NOT_FOUND_ERROR

if (ssg404) {
try {
;({ page: Component, styleSheets } = await this.fetchComponent(
let mod: any
;({ page: Component, styleSheets, mod } = await this.fetchComponent(
'/404'
))

// TODO: should we tolerate these props missing and still render the
// page instead of falling back to _error?
if (mod && mod.__N_SSG) {
props = await this._getStaticData(
this.pageLoader.getDataHref('/404', '/404', true, this.locale)
)
}
} catch (_err) {
// non-fatal fallback to _error
}
Expand All @@ -958,26 +968,24 @@ export default class Router implements BaseRouter {
}

const routeInfo: PrivateRouteInfo = {
props,
Component,
styleSheets,
err: ssg404 ? undefined : err,
error: ssg404 ? undefined : err,
}

try {
routeInfo.props = await this.getInitialProps(Component, {
err,
pathname,
query,
} as any)

if (ssg404 && routeInfo.props && routeInfo.props.pageProps) {
routeInfo.props.pageProps.statusCode = 404
if (!routeInfo.props) {
try {
routeInfo.props = await this.getInitialProps(Component, {
err,
pathname,
query,
} as any)
} catch (gipErr) {
console.error('Error in error page `getInitialProps`: ', gipErr)
routeInfo.props = {}
}
console.log(routeInfo)
} catch (gipErr) {
console.error('Error in error page `getInitialProps`: ', gipErr)
routeInfo.props = {}
}

return routeInfo
Expand Down
25 changes: 12 additions & 13 deletions packages/next/next-server/server/next-server.ts
Expand Up @@ -1185,8 +1185,19 @@ export default class Server {
): Promise<string | null> {
const is404Page = pathname === '/404'

const isLikeServerless =
typeof components.Component === 'object' &&
typeof (components.Component as any).renderReqToHTML === 'function'
const isSSG = !!components.getStaticProps
const isServerProps = !!components.getServerSideProps
const hasStaticPaths = !!components.getStaticPaths

// Toggle whether or not this is a Data request
const isDataReq = !!query._nextDataReq && (isSSG || isServerProps)
delete query._nextDataReq

// we need to ensure the status code if /404 is visited directly
if (is404Page) {
if (is404Page && !isDataReq) {
res.statusCode = 404
}

Expand All @@ -1195,22 +1206,10 @@ export default class Server {
return components.Component
}

// check request state
const isLikeServerless =
typeof components.Component === 'object' &&
typeof (components.Component as any).renderReqToHTML === 'function'
const isSSG = !!components.getStaticProps
const isServerProps = !!components.getServerSideProps
const hasStaticPaths = !!components.getStaticPaths

if (!query.amp) {
delete query.amp
}

// Toggle whether or not this is a Data request
const isDataReq = !!query._nextDataReq && (isSSG || isServerProps)
delete query._nextDataReq

const locale = query.__nextLocale as string
delete query.__nextLocale

Expand Down
5 changes: 3 additions & 2 deletions test/integration/i18n-support/pages/404.js
Expand Up @@ -2,14 +2,15 @@ export default function NotFound(props) {
return (
<>
<h1 id="not-found">This page could not be found | 404</h1>
<p id="prop">{JSON.stringify(props)}</p>
<p id="props">{JSON.stringify(props)}</p>
</>
)
}

export const getStaticProps = () => {
export const getStaticProps = ({ locale }) => {
return {
props: {
locale,
is404: true,
},
}
Expand Down
44 changes: 44 additions & 0 deletions test/integration/i18n-support/test/index.test.js
Expand Up @@ -824,6 +824,11 @@ function runTests(isDev) {
await browser.eval('document.documentElement.innerHTML')
).toContain('This page could not be found')

const props = JSON.parse(await browser.elementByCss('#props').text())

expect(props.is404).toBe(true)
expect(props.locale).toBe(locale)

const parsedUrl = url.parse(
await browser.eval('window.location.href'),
true
Expand All @@ -847,9 +852,44 @@ function runTests(isDev) {
expect(await browser.elementByCss('html').text()).toContain(
'This page could not be found'
)
const props = JSON.parse(await browser.elementByCss('#props').text())

expect(props.is404).toBe(true)
expect(props.locale).toBe('en')
expect(await browser.eval('window.beforeNav')).toBe(null)
})

it('should render 404 for fallback page that returned 404 on client transition', async () => {
const browser = await webdriver(appPort, '/en', true, true)
await browser.eval(`(function() {
next.router.push('/not-found/fallback/first')
})()`)
await browser.waitForElementByCss('h1')
await browser.eval('window.beforeNav = 1')

expect(await browser.elementByCss('html').text()).toContain(
'This page could not be found'
)
const props = JSON.parse(await browser.elementByCss('#props').text())

expect(props.is404).toBe(true)
expect(props.locale).toBe('en')
expect(await browser.elementByCss('html').getAttribute('lang')).toBe('en')

const parsedUrl = url.parse(
await browser.eval('window.location.href'),
true
)
expect(parsedUrl.pathname).toBe('/en/not-found/fallback/first')
expect(parsedUrl.query).toEqual({})

if (isDev) {
// make sure page doesn't reload un-necessarily in development
await waitFor(10 * 1000)
}
expect(await browser.eval('window.beforeNav')).toBe(1)
})

it('should render 404 for fallback page that returned 404', async () => {
const browser = await webdriver(
appPort,
Expand All @@ -863,6 +903,10 @@ function runTests(isDev) {
expect(await browser.elementByCss('html').text()).toContain(
'This page could not be found'
)
const props = JSON.parse(await browser.elementByCss('#props').text())

expect(props.is404).toBe(true)
expect(props.locale).toBe('en')
expect(await browser.elementByCss('html').getAttribute('lang')).toBe('en')

const parsedUrl = url.parse(
Expand Down