Skip to content

Commit

Permalink
Adjust SSG Loading Behavior (#10510)
Browse files Browse the repository at this point in the history 
* Adjust SSG Loading Behavior

* Update expected preview behavior

* Rename two corrections

* Only use skeleton in production for now

* Fix "should SSR SPR page correctly" test

* fix tests

* fix trailing comment letter

* disable test for now
  • Loading branch information
@Timer
Timer committed Feb 13, 2020
1 parent 5f04144 commit e38e3dd
Show file tree
Hide file tree
Showing 7 changed files with 139 additions and 157 deletions.
11 changes: 7 additions & 4 deletions packages/next/build/index.ts
View file
Expand Up @@ -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 } }
}
})
Expand Down
11 changes: 8 additions & 3 deletions packages/next/client/index.js
View file
Expand Up @@ -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 ||
Expand All @@ -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,
}
)
Expand Down
155 changes: 54 additions & 101 deletions packages/next/next-server/lib/router/router.ts
View file
Expand Up @@ -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__
Expand All @@ -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 {
Expand Down Expand Up @@ -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)
})
}
Expand Down Expand Up @@ -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 => {
Expand Down Expand Up @@ -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(
Expand Down
40 changes: 30 additions & 10 deletions packages/next/next-server/server/next-server.ts
View file
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions test/integration/dynamic-routing/test/index.test.js
View file
Expand Up @@ -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`
Expand Down
10 changes: 5 additions & 5 deletions test/integration/prerender-preview/test/index.test.js
View file
Expand Up @@ -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,
'/',
Expand All @@ -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 () => {
Expand Down Expand Up @@ -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"}'
)
Expand Down

0 comments on commit e38e3dd

Please sign in to comment.