Add fetching 404 SSG data on fallback notFound

vercel · Oct 25, 2020 · 9804c5f · 9804c5f
1 parent 6d3b065
commit 9804c5f
Show file tree

Hide file tree

Showing 4 changed files with 81 additions and 29 deletions.
diff --git a/packages/next/next-server/lib/router/router.ts b/packages/next/next-server/lib/router/router.ts
@@ -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
         }
@@ -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

diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts
@@ -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
     }
 
@@ -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
 

diff --git a/test/integration/i18n-support/pages/404.js b/test/integration/i18n-support/pages/404.js
@@ -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,
     },
   }

diff --git a/test/integration/i18n-support/test/index.test.js b/test/integration/i18n-support/test/index.test.js
@@ -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
@@ -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,
@@ -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(