Add additional tests and update handling for dev

packages/next/next-server/lib/router/router.ts

@@ -298,6 +298,8 @@ const manualScrollRestoration =
   typeof window !== 'undefined' &&
   'scrollRestoration' in window.history
 
+const SSG_DATA_NOT_FOUND_ERROR = 'SSG Data NOT_FOUND'
+
 function fetchRetry(url: string, attempts: number): Promise<any> {
   return fetch(url, {
     // Cookies are required to be present for Next.js' SSG "Preview Mode".
@@ -320,7 +322,7 @@ function fetchRetry(url: string, attempts: number): Promise<any> {
       if (res.status === 404) {
         // TODO: handle reloading in development from fallback returning 200
         // to on-demand-entry-handler causing it to reload periodically
-        throw new Error('NOT_FOUND')
+        throw new Error(SSG_DATA_NOT_FOUND_ERROR)
       }
       throw new Error(`Failed to load static props`)
     }
@@ -334,7 +336,7 @@ function fetchNextData(dataHref: string, isServerRender: boolean) {
     // on a client-side transition. Otherwise, we'd get into an infinite
     // loop.
 
-    if (!isServerRender || err.message === 'NOT_FOUND') {
+    if (!isServerRender || err.message === 'SSG Data NOT_FOUND') {
       markLoadingError(err)
     }
     throw err
@@ -903,6 +905,13 @@ export default class Router implements BaseRouter {
       //  3. Internal error while loading the page
 
       // So, doing a hard reload is the proper way to deal with this.
+      if (process.env.NODE_ENV === 'development') {
+        // append __next404 query to prevent fallback from being re-served
+        // on reload in development
+        if (err.message === SSG_DATA_NOT_FOUND_ERROR && this.isSsr) {
+          as += `${as.indexOf('?') > -1 ? '&' : '?'}__next404=1`
+        }
+      }
       window.location.href = as
 
       // Changing the URL doesn't block executing the current code path.

packages/next/next-server/server/next-server.ts

@@ -149,7 +149,6 @@ export default class Server {
   router: Router
   protected dynamicRoutes?: DynamicRoutes
   protected customRoutes: CustomRoutes
-  protected pendingSsg404s?: Set<string>
 
   public constructor({
     dir = '.',
@@ -1077,6 +1076,7 @@ export default class Server {
             ...(components.getStaticProps
               ? {
                   amp: query.amp,
+                  __next404: query.__next404,
                   _nextDataReq: query._nextDataReq,
                   __nextLocale: query.__nextLocale,
                   __nextLocales: query.__nextLocales,
@@ -1151,6 +1151,13 @@ export default class Server {
     const isDataReq = !!query._nextDataReq && (isSSG || isServerProps)
     delete query._nextDataReq
 
+    const locale = query.__nextLocale as string
+    const locales = query.__nextLocales as string[]
+    // const defaultLocale = query.__nextDefaultLocale as string
+    delete query.__nextLocale
+    delete query.__nextLocales
+    // delete query.__nextDefaultLocale
+
     let previewData: string | false | object | undefined
     let isPreviewMode = false
 
@@ -1179,7 +1186,7 @@ export default class Server {
       }
 
       if (this.nextConfig.experimental.i18n) {
-        return normalizeLocalePath(path, this.renderOpts.locales).pathname
+        return normalizeLocalePath(path, locales).pathname
       }
       return path
     }
@@ -1191,30 +1198,19 @@ export default class Server {
       urlPathname = stripNextDataPath(urlPathname)
     }
 
-    const locale = query.__nextLocale as string
-    const locales = query.__nextLocales as string[]
-    // const defaultLocale = query.__nextDefaultLocale as string
-    delete query.__nextLocale
-    delete query.__nextLocales
-    // delete query.__nextDefaultLocale
-
     const ssgCacheKey =
       isPreviewMode || !isSSG
         ? undefined // Preview mode bypasses the cache
         : `${locale ? `/${locale}` : ''}${resolvedUrlPathname}${
             query.amp ? '.amp' : ''
           }`
 
-    // in development handle 404 before re-serving fallback
-    // since the page reloads when the data 404s and now
-    // we need to render the 404, in production this is
-    // populated in the incremental cache which is a no-op in dev
-    if (
-      ssgCacheKey &&
-      this.renderOpts.dev &&
-      this.pendingSsg404s?.has(ssgCacheKey)
-    ) {
-      this.pendingSsg404s.delete(ssgCacheKey)
+    // In development we use a __next404 query to allow signaling we should
+    // render the 404 page after attempting to fetch the _next/data for a
+    // fallback page since the fallback page will always be available after
+    // reload and we don't want to re-serve it and instead want to 404.
+    if (this.renderOpts.dev && isSSG && query.__next404) {
+      delete query.__next404
       throw new NoFallbackError()
     }
 
@@ -1296,11 +1292,6 @@ export default class Server {
             }
           )
 
-          if (renderResult.renderOpts.ssgNotFound) {
-            // trigger 404
-            throw new NoFallbackError()
-          }
-
           html = renderResult.html
           pageData = renderResult.renderOpts.pageData
           sprRevalidate = renderResult.renderOpts.revalidate
@@ -1470,13 +1461,8 @@ export default class Server {
     }
 
     if (isNotFound) {
-      // trigger 404
-      if (ssgCacheKey && this.renderOpts.dev && this.pendingSsg404s) {
-        this.pendingSsg404s.add(ssgCacheKey)
-      }
       throw new NoFallbackError()
     }
-
     return resHtml
   }
 

packages/next/next-server/server/render.tsx

@@ -416,6 +416,7 @@ export async function renderToHTML(
   delete query.__nextLocale
   delete query.__nextLocales
   delete query.__nextDefaultLocale
+  delete query.__next404
 
   const isSSG = !!getStaticProps
   const isBuildTimeSSG = isSSG && renderOpts.nextExport
@@ -640,6 +641,7 @@ export async function renderToHTML(
 
       if (data.unstable_notFound) {
         ;(renderOpts as any).ssgNotFound = true
+        ;(renderOpts as any).revalidate = false
         return null
       }
 

packages/next/server/next-dev-server.ts

@@ -55,7 +55,6 @@ export default class DevServer extends Server {
   protected staticPathsWorker: import('jest-worker').default & {
     loadStaticPaths: typeof import('./static-paths-worker').loadStaticPaths
   }
-  protected pendingSsg404s: Set<string>
 
   constructor(options: ServerConstructor & { isNextDevCommand?: boolean }) {
     super({ ...options, dev: true })
@@ -114,8 +113,6 @@ export default class DevServer extends Server {
 
     this.staticPathsWorker.getStdout().pipe(process.stdout)
     this.staticPathsWorker.getStderr().pipe(process.stderr)
-
-    this.pendingSsg404s = new Set()
   }
 
   protected currentPhase(): string {

test/integration/i18n-support/test/index.test.js

@@ -14,6 +14,7 @@ import {
   nextStart,
   renderViaHTTP,
   File,
+  waitFor,
 } from 'next-test-utils'
 
 jest.setTimeout(1000 * 60 * 2)
@@ -206,10 +207,65 @@ function runTests(isDev) {
     for (const locale of locales) {
       const res = await fetchViaHTTP(appPort, `/${locale}/not-found`)
       expect(res.status).toBe(skippedLocales.includes(locale) ? 404 : 200)
+
+      if (skippedLocales.includes(locale)) {
+        const browser = await webdriver(appPort, `/${locale}/not-found`)
+        expect(await browser.elementByCss('html').getAttribute('lang')).toBe(
+          locale
+        )
+        expect(
+          await browser.eval('document.documentElement.innerHTML')
+        ).toContain('This page could not be found')
+
+        const parsedUrl = url.parse(
+          await browser.eval('window.location.href'),
+          true
+        )
+        expect(parsedUrl.pathname).toBe(`/${locale}/not-found`)
+        expect(parsedUrl.query).toEqual({})
+      }
     }
   })
 
-  // TODO: tests for notFound with fallback GSP pages
+  it('should 404 for GSP that returned notFound on client-transition', async () => {
+    const browser = await webdriver(appPort, '/en')
+    await browser.eval(`(function() {
+      window.beforeNav = 1
+      window.next.router.push('/not-found')
+    })()`)
+
+    await browser.waitForElementByCss('h1')
+
+    expect(await browser.elementByCss('html').getAttribute('lang')).toBe('en')
+    expect(await browser.elementByCss('html').text()).toContain(
+      'This page could not be found'
+    )
+    expect(await browser.eval('window.beforeNav')).toBe(null)
+  })
+
+  it('should render 404 for fallback page that returned 404', async () => {
+    const browser = await webdriver(appPort, '/en/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'
+    )
+    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(isDev ? { __next404: '1' } : {})
+
+    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 remove un-necessary locale prefix for default locale', async () => {
     const res = await fetchViaHTTP(appPort, '/en-US', undefined, {