Adjust SSG Loading Behavior

packages/next/build/index.ts

@@ -657,15 +657,18 @@ export default async function build(dir: string, conf = null): Promise<void> {
       // n.b. we cannot handle this above in combinedPages because the dynamic
       // page must be in the `pages` array, but not in the mapping.
       exportPathMap: (defaultMap: any) => {
-        // Generate fallback for dynamically routed pages to use as
-        // the loading state for pages while the data is being populated
+        // Dynamically routed pages should be prerendered to be used as
+        // a client-side skeleton (fallback) while data is being fetched.
+        // This ensures the end-user never sees a 500 or slow response from the
+        // server.
         //
         // Note: prerendering disables automatic static optimization.
         ssgPages.forEach(page => {
           if (isDynamicRoute(page)) {
             tbdPrerenderRoutes.push(page)
-            // set __nextFallback query so render doesn't call
-            // getStaticProps/getServerProps
+
+            // Override the rendering for the dynamic page to be treated as a
+            // fallback render.
             defaultMap[page] = { page, query: { __nextFallback: true } }
           }
         })

packages/next/client/index.js

@@ -97,9 +97,10 @@ class Container extends React.Component {
         })
     }
 
-    // If page was exported and has a querystring
-    // If it's a dynamic route or has a querystring
-    // if it's a fallback page
+    // We need to replace the router state if:
+    // - the page was (auto) exported and has a query string or search (hash)
+    // - it was auto exported and is a dynamic route (to provide params)
+    // - if it is a client-side skeleton (fallback render)
     if (
       router.isSsr &&
       (isFallback ||
@@ -121,6 +122,10 @@ class Container extends React.Component {
           // client-side hydration. Your app should _never_ use this property.
           // It may change at any time without notice.
           _h: 1,
+          // Fallback pages must trigger the data fetch, so the transition is
+          // not shallow.
+          // Other pages (strictly updating query) happens shallowly, as data
+          // requirements would already be present.
           shallow: !isFallback,
         }
       )

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

@@ -64,11 +64,12 @@ type BeforePopStateCallback = (state: any) => boolean
 
 type ComponentLoadCancel = (() => void) | null
 
-const fetchNextData = (
+function fetchNextData(
   pathname: string,
   query: ParsedUrlQuery | null,
+  isServerRender: boolean,
   cb?: (...args: any) => any
-) => {
+) {
   return fetch(
     formatWithValidation({
       // @ts-ignore __NEXT_DATA__
@@ -78,15 +79,22 @@ const fetchNextData = (
   )
     .then(res => {
       if (!res.ok) {
-        const error = new Error(`Failed to load static props`)
-        ;(error as any).statusCode = res.status
-        throw error
+        throw new Error(`Failed to load static props`)
       }
       return res.json()
     })
     .then(data => {
       return cb ? cb(data) : data
     })
+    .catch((err: Error) => {
+      // We should only trigger a server-side transition if this was caused
+      // on a client-side transition. Otherwise, we'd get into an infinite
+      // loop.
+      if (!isServerRender) {
+        ;(err as any).code = 'PAGE_LOAD_ERROR'
+      }
+      throw err
+    })
 }
 
 export default class Router implements BaseRouter {
@@ -391,65 +399,31 @@ export default class Router implements BaseRouter {
 
       // If shallow is true and the route exists in the router cache we reuse the previous result
       this.getRouteInfo(route, pathname, query, as, shallow).then(routeInfo => {
-        let emitHistory = false
-
-        const doRouteChange = (routeInfo: RouteInfo, complete: boolean) => {
-          const { error } = routeInfo
-
-          if (error && error.cancelled) {
-            return resolve(false)
-          }
+        const { error } = routeInfo
 
-          if (!emitHistory) {
-            emitHistory = true
-            Router.events.emit('beforeHistoryChange', as)
-            this.changeState(method, url, addBasePath(as), options)
-          }
+        if (error && error.cancelled) {
+          return resolve(false)
+        }
 
-          if (process.env.NODE_ENV !== 'production') {
-            const appComp: any = this.components['/_app'].Component
-            ;(window as any).next.isPrerendered =
-              appComp.getInitialProps === appComp.origGetInitialProps &&
-              !(routeInfo.Component as any).getInitialProps
-          }
+        Router.events.emit('beforeHistoryChange', as)
+        this.changeState(method, url, addBasePath(as), options)
 
-          this.set(route, pathname, query, as, routeInfo)
+        if (process.env.NODE_ENV !== 'production') {
+          const appComp: any = this.components['/_app'].Component
+          ;(window as any).next.isPrerendered =
+            appComp.getInitialProps === appComp.origGetInitialProps &&
+            !(routeInfo.Component as any).getInitialProps
+        }
 
-          if (complete) {
-            if (error) {
-              Router.events.emit('routeChangeError', error, as)
-              throw error
-            }
+        this.set(route, pathname, query, as, routeInfo)
 
-            Router.events.emit('routeChangeComplete', as)
-            resolve(true)
-          }
+        if (error) {
+          Router.events.emit('routeChangeError', error, as)
+          throw error
         }
 
-        if ((routeInfo as any).dataRes) {
-          const dataRes = (routeInfo as any).dataRes as Promise<RouteInfo>
-
-          // to prevent a flash of the fallback page we delay showing it for
-          // 110ms and race the timeout with the data response. If the data
-          // beats the timeout we skip showing the fallback
-          Promise.race([
-            new Promise(resolve => setTimeout(() => resolve(false), 110)),
-            dataRes,
-          ])
-            .then((data: any) => {
-              if (!data) {
-                // data didn't win the race, show fallback
-                doRouteChange(routeInfo, false)
-              }
-              return dataRes
-            })
-            .then(finalData => {
-              // render with the data and complete route change
-              doRouteChange(finalData as RouteInfo, true)
-            }, reject)
-        } else {
-          doRouteChange(routeInfo, true)
-        }
+        Router.events.emit('routeChangeComplete', as)
+        return resolve(true)
       }, reject)
     })
   }
@@ -518,51 +492,25 @@ export default class Router implements BaseRouter {
           }
         }
 
-        const isSSG = (Component as any).__N_SSG
-        const isSSP = (Component as any).__N_SSP
-
-        const handleData = (props: any) => {
+        return this._getData<RouteInfo>(() =>
+          (Component as any).__N_SSG
+            ? this._getStaticData(as)
+            : (Component as any).__N_SSP
+            ? this._getServerData(as)
+            : this.getInitialProps(
+                Component,
+                // we provide AppTree later so this needs to be `any`
+                {
+                  pathname,
+                  query,
+                  asPath: as,
+                } as any
+              )
+        ).then(props => {
           routeInfo.props = props
           this.components[route] = routeInfo
           return routeInfo
-        }
-
-        // resolve with fallback routeInfo and promise for data
-        if (isSSG || isSSP) {
-          const dataMethod = () =>
-            isSSG ? this._getStaticData(as) : this._getServerData(as)
-
-          const retry = (error: Error & { statusCode: number }) => {
-            if (error.statusCode === 404) {
-              throw error
-            }
-            return dataMethod()
-          }
-
-          return Promise.resolve({
-            ...routeInfo,
-            props: {},
-            dataRes: this._getData(() =>
-              dataMethod()
-                // we retry for data twice unless we get a 404
-                .catch(retry)
-                .catch(retry)
-                .then((props: any) => handleData(props))
-            ),
-          })
-        }
-
-        return this._getData<RouteInfo>(() =>
-          this.getInitialProps(
-            Component,
-            // we provide AppTree later so this needs to be `any`
-            {
-              pathname,
-              query,
-              asPath: as,
-            } as any
-          )
-        ).then(props => handleData(props))
+        })
       })
       .catch(err => {
         return new Promise(resolve => {
@@ -765,13 +713,18 @@ export default class Router implements BaseRouter {
 
     return process.env.NODE_ENV === 'production' && this.sdc[pathname]
       ? Promise.resolve(this.sdc[pathname])
-      : fetchNextData(pathname, null, data => (this.sdc[pathname] = data))
+      : fetchNextData(
+          pathname,
+          null,
+          this.isSsr,
+          data => (this.sdc[pathname] = data)
+        )
   }
 
   _getServerData = (asPath: string): Promise<object> => {
     let { pathname, query } = parse(asPath, true)
     pathname = prepareRoute(pathname!)
-    return fetchNextData(pathname, query)
+    return fetchNextData(pathname, query, this.isSsr)
   }
 
   getInitialProps(

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

@@ -992,22 +992,42 @@ export default class Server {
       return { html, pageData, sprRevalidate }
     })
 
-    // render fallback if for a preview path or a non-seeded dynamic path
+    const isProduction = !this.renderOpts.dev
     const isDynamicPathname = isDynamicRoute(pathname)
+    const didRespond = isResSent(res)
+    // const isForcedBlocking =
+    //   req.headers['X-Prerender-Bypass-Mode'] !== 'Blocking'
+
+    // When we did not respond from cache, we need to choose to block on
+    // rendering or return a skeleton.
+    //
+    // * Data requests always block.
+    //
+    // * Preview mode toggles all pages to be resolved in a blocking manner.
+    //
+    // * Non-dynamic pages should block (though this is an be an impossible
+    //   case in production).
+    //
+    // * Dynamic pages should return their skeleton, then finish the data
+    //   request on the client-side.
+    //
     if (
-      !isResSent(res) &&
+      !didRespond &&
       !isDataReq &&
-      ((isPreviewMode &&
-        // A header can opt into the blocking behavior.
-        req.headers['X-Prerender-Bypass-Mode'] !== 'Blocking') ||
-        isDynamicPathname)
+      !isPreviewMode &&
+      isDynamicPathname &&
+      // TODO: development should trigger fallback when the path is not in
+      // `getStaticPaths`, for now, let's assume it is.
+      isProduction
     ) {
-      let html = ''
+      let html: string
 
-      const isProduction = !this.renderOpts.dev
-      if (isProduction && (isDynamicPathname || !isPreviewMode)) {
+      // Production already emitted the fallback as static HTML.
+      if (isProduction) {
         html = await getFallback(pathname)
-      } else {
+      }
+      // We need to generate the fallback on-demand for development.
+      else {
         query.__nextFallback = 'true'
         if (isLikeServerless) {
           this.prepareServerlessUrl(req, query)

test/integration/dynamic-routing/test/index.test.js

@@ -275,15 +275,15 @@ function runTests(dev) {
     })
   })
 
-  it('[predefined ssg: prerendered catch all] should pass param in getInitialProps during SSR', async () => {
+  it('[predefined ssg: prerendered catch all] should pass param in getStaticProps during SSR', async () => {
     const data = await renderViaHTTP(
       appPort,
       `/_next/data/${buildId}/p1/p2/predefined-ssg/one-level.json`
     )
     expect(JSON.parse(data).pageProps.params).toEqual({ rest: ['one-level'] })
   })
 
-  it('[predefined ssg: prerendered catch all] should pass params in getInitialProps during SSR', async () => {
+  it('[predefined ssg: prerendered catch all] should pass params in getStaticProps during SSR', async () => {
     const data = await renderViaHTTP(
       appPort,
       `/_next/data/${buildId}/p1/p2/predefined-ssg/1st-level/2nd-level.json`

test/integration/prerender-preview/test/index.test.js

@@ -81,7 +81,7 @@ function runTests() {
       cookie.serialize('__next_preview_data', cookies[1].__next_preview_data)
   })
 
-  it('should return fallback page on preview request', async () => {
+  it('should not return fallback page on preview request', async () => {
     const res = await fetchViaHTTP(
       appPort,
       '/',
@@ -91,8 +91,8 @@ function runTests() {
     const html = await res.text()
 
     const { nextData, pre } = getData(html)
-    expect(nextData).toMatchObject({ isFallback: true })
-    expect(pre).toBe('Has No Props')
+    expect(nextData).toMatchObject({ isFallback: false })
+    expect(pre).toBe('true and {"lets":"goooo"}')
   })
 
   it('should return cookies to be expired on reset request', async () => {
@@ -136,8 +136,8 @@ function runTests() {
   it('should fetch preview data', async () => {
     await browser.get(`http://localhost:${appPort}/`)
     await browser.waitForElementByCss('#props-pre')
-    expect(await browser.elementById('props-pre').text()).toBe('Has No Props')
-    await new Promise(resolve => setTimeout(resolve, 2000))
+    // expect(await browser.elementById('props-pre').text()).toBe('Has No Props')
+    // await new Promise(resolve => setTimeout(resolve, 2000))
     expect(await browser.elementById('props-pre').text()).toBe(
       'true and {"client":"mode"}'
     )