Add retrying for data, de-dupe initial fallback request, and merge fa…

…llback replace
vercel · Feb 7, 2020 · 66e7e51 · 66e7e51
1 parent cebfcbc
commit 66e7e51
Show file tree

Hide file tree

Showing 2 changed files with 63 additions and 59 deletions.
diff --git a/packages/next/client/index.js b/packages/next/client/index.js
@@ -44,8 +44,11 @@ const {
  assetPrefix,
  runtimeConfig,
  dynamicIds,
+ isFallback,
 } = data
 
+let fallbackDataPromise = Promise.resolve()
+
 const prefix = assetPrefix || ''
 
 // With dynamic assetPrefix it's no longer possible to set assetPrefix at the build time
@@ -96,51 +99,38 @@ class Container extends React.Component {
  })
  }
 
- // call router.replace to trigger data fetching while
- // the fallback is shown
-
- if (data.isFallback) {
- router.replace(
- {
- pathname: page,
- query: {
- ...router.query,
- ...parseQs(location.search.substr(1)),
- },
- },
- asPath,
- { _h: 1 }
- )
- }
-
- // If page was exported and has a querystring
- // If it's a dynamic route or has a querystring
-
- if (
- !data.isFallback &&
- router.isSsr &&
- ((data.nextExport &&
- (isDynamicRoute(router.pathname) || location.search)) ||
- (Component && Component.__N_SSG && location.search))
- ) {
- // update query on mount for exported pages
- router.replace(
- router.pathname +
- '?' +
- stringifyQs({
- ...router.query,
- ...parseQs(location.search.substr(1)),
- }),
- asPath,
- {
- // WARNING: `_h` is an internal option for handing Next.js
- // client-side hydration. Your app should _never_ use this property.
- // It may change at any time without notice.
- _h: 1,
- shallow: true,
- }
- )
- }
+ // wait for fallbackDataPromise so we don't kick off an extra
+ // request if one is pending
+ fallbackDataPromise.then(() => {
+ // If page was exported and has a querystring
+ // If it's a dynamic route or has a querystring
+ // if it's a fallback page
+ if (
+ router.isSsr &&
+ (isFallback ||
+ (data.nextExport &&
+ (isDynamicRoute(router.pathname) || location.search)) ||
+ (Component && Component.__N_SSG && location.search))
+ ) {
+ // update query on mount for exported pages
+ router.replace(
+ router.pathname +
+ '?' +
+ stringifyQs({
+ ...router.query,
+ ...parseQs(location.search.substr(1)),
+ }),
+ asPath,
+ {
+ // WARNING: `_h` is an internal option for handing Next.js
+ // client-side hydration. Your app should _never_ use this property.
+ // It may change at any time without notice.
+ _h: 1,
+ shallow: !isFallback,
+ }
+ )
+ }
+ })
 
  if (process.env.__NEXT_TEST_MODE) {
  window.__NEXT_HYDRATED = true
@@ -240,7 +230,7 @@ export default async ({ webpackHMR: passedWebpackHMR } = {}) => {
  // kick off static data request now so it's in the cache
  // when we re-render post-hydration
  if (data.isFallback) {
- router._getStaticData(asPath).catch(() => {})
+ fallbackDataPromise = router._getStaticData(asPath).catch(() => {})
  }
 
  render(renderCtx)

diff --git a/packages/next/next-server/lib/router/router.ts b/packages/next/next-server/lib/router/router.ts
@@ -78,17 +78,15 @@ const fetchNextData = (
  )
  .then(res => {
  if (!res.ok) {
- throw new Error(`Failed to load static props`)
+ const error = new Error(`Failed to load static props`)
+ ;(error as any).statusCode = res.status
+ throw error
  }
  return res.json()
  })
  .then(data => {
  return cb ? cb(data) : data
  })
- .catch((err: Error) => {
- ;(err as any).code = 'PAGE_LOAD_ERROR'
- throw err
- })
 }
 
 export default class Router implements BaseRouter {
@@ -393,15 +391,20 @@ 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 => {
- const doRouteChange = (routeInfo: RouteInfo, emit: boolean) => {
+ let emitHistory = false
+
+ const doRouteChange = (routeInfo: RouteInfo, complete: boolean) => {
  const { error } = routeInfo
 
  if (error && error.cancelled) {
  return resolve(false)
  }
 
- Router.events.emit('beforeHistoryChange', as)
- this.changeState(method, url, addBasePath(as), options)
+ if (!emitHistory) {
+ emitHistory = true
+ Router.events.emit('beforeHistoryChange', as)
+ this.changeState(method, url, addBasePath(as), options)
+ }
 
  if (process.env.NODE_ENV !== 'production') {
  const appComp: any = this.components['/_app'].Component
@@ -412,7 +415,7 @@ export default class Router implements BaseRouter {
 
  this.set(route, pathname, query, as, routeInfo)
 
- if (emit) {
+ if (complete) {
  if (error) {
  Router.events.emit('routeChangeError', error, as)
  throw error
@@ -526,14 +529,25 @@ export default class Router implements BaseRouter {
 
  // 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(() =>
- (isSSG
- ? this._getStaticData(as)
- : this._getServerData(as)
- ).then((props: any) => handleData(props))
+ dataMethod()
+ // we retry for data twice unless we get a 404
+ .catch(retry)
+ .catch(retry)
+ .then((props: any) => handleData(props))
  ),
  })
  }