diff --git a/packages/next/build/index.ts b/packages/next/build/index.ts index 02f6747d8b67034..f95e028f64dd8e7 100644 --- a/packages/next/build/index.ts +++ b/packages/next/build/index.ts @@ -657,15 +657,18 @@ export default async function build(dir: string, conf = null): Promise { // 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 } } } }) diff --git a/packages/next/client/index.js b/packages/next/client/index.js index 9bf4edbded0a1e2..ffe66c971534055 100644 --- a/packages/next/client/index.js +++ b/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, } ) diff --git a/packages/next/next-server/lib/router/router.ts b/packages/next/next-server/lib/router/router.ts index d658f49836270ac..e2f45975414f09b 100644 --- a/packages/next/next-server/lib/router/router.ts +++ b/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 - - // 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(() => + (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(() => - 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 => { let { pathname, query } = parse(asPath, true) pathname = prepareRoute(pathname!) - return fetchNextData(pathname, query) + return fetchNextData(pathname, query, this.isSsr) } getInitialProps( diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index 5dd9b0ac19d3f02..29e875557b80df6 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/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) diff --git a/test/integration/dynamic-routing/test/index.test.js b/test/integration/dynamic-routing/test/index.test.js index 375092823b0e099..0b3d8f125d260fd 100644 --- a/test/integration/dynamic-routing/test/index.test.js +++ b/test/integration/dynamic-routing/test/index.test.js @@ -275,7 +275,7 @@ 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` @@ -283,7 +283,7 @@ function runTests(dev) { 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` diff --git a/test/integration/prerender-preview/test/index.test.js b/test/integration/prerender-preview/test/index.test.js index af78b60b0739558..120be2cef3c594a 100644 --- a/test/integration/prerender-preview/test/index.test.js +++ b/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"}' ) diff --git a/test/integration/prerender/test/index.test.js b/test/integration/prerender/test/index.test.js index c2d98dbf756950e..ba34b8a31d55741 100644 --- a/test/integration/prerender/test/index.test.js +++ b/test/integration/prerender/test/index.test.js @@ -254,12 +254,9 @@ const runTests = (dev = false) => { it('should SSR SPR page correctly', async () => { const html = await renderViaHTTP(appPort, '/blog/post-1') - if (dev) { - const $ = cheerio.load(html) - expect(JSON.parse($('#__NEXT_DATA__').text()).isFallback).toBe(true) - } else { - expect(html).toMatch(/Post:.*?post-1/) - } + const $ = cheerio.load(html) + expect(JSON.parse($('#__NEXT_DATA__').text()).isFallback).toBe(false) + expect(html).toMatch(/Post:.*?post-1/) }) it('should not supply query values to params or useRouter non-dynamic page SSR', async () => { @@ -285,11 +282,8 @@ const runTests = (dev = false) => { const html = await renderViaHTTP(appPort, '/blog/post-1?hello=world') const $ = cheerio.load(html) - if (!dev) { - // these aren't available in dev since we render the fallback always - const params = $('#params').text() - expect(JSON.parse(params)).toEqual({ post: 'post-1' }) - } + const params = $('#params').text() + expect(JSON.parse(params)).toEqual({ post: 'post-1' }) const query = $('#query').text() expect(JSON.parse(query)).toEqual({ post: 'post-1' }) @@ -358,33 +352,40 @@ const runTests = (dev = false) => { const html = await renderViaHTTP(appPort, '/catchall/another/value') const $ = cheerio.load(html) - if (dev) { - expect( - JSON.parse( - cheerio - .load(html)('#__NEXT_DATA__') - .text() - ).isFallback - ).toBe(true) - } else { - expect($('#catchall').text()).toMatch(/Hi.*?another\/value/) - } + expect( + JSON.parse( + cheerio + .load(html)('#__NEXT_DATA__') + .text() + ).isFallback + ).toBe(false) + expect($('#catchall').text()).toMatch(/Hi.*?another\/value/) }) it('should support lazy catchall route', async () => { - const browser = await webdriver(appPort, '/catchall/third') - const text = await browser.elementByCss('#catchall').text() - expect(text).toMatch(/Hi.*?third/) + // Dev doesn't support fallback yet + if (dev) { + const html = await renderViaHTTP(appPort, '/catchall/notreturnedinpaths') + const $ = cheerio.load(html) + expect($('#catchall').text()).toMatch(/Hi.*?notreturnedinpaths/) + } + // Production will render fallback for a "lazy" route + else { + const browser = await webdriver(appPort, '/catchall/notreturnedinpaths') + const text = await browser.elementByCss('#catchall').text() + expect(text).toMatch(/Hi.*?notreturnedinpaths/) + } }) if (dev) { - it('should show error when rewriting to dynamic SSG page', async () => { - const item = Math.round(Math.random() * 100) - const html = await renderViaHTTP(appPort, `/some-rewrite/${item}`) - expect(html).toContain( - `Rewrites don't support dynamic pages with getStaticProps yet. Using this will cause the page to fail to parse the params on the client for the fallback page` - ) - }) + // TODO: re-enable when this is supported in dev + // it('should show error when rewriting to dynamic SSG page', async () => { + // const item = Math.round(Math.random() * 100) + // const html = await renderViaHTTP(appPort, `/some-rewrite/${item}`) + // expect(html).toContain( + // `Rewrites don't support dynamic pages with getStaticProps yet. Using this will cause the page to fail to parse the params on the client for the fallback page` + // ) + // }) it('should always call getStaticProps without caching in dev', async () => { const initialRes = await fetchViaHTTP(appPort, '/something')