From 1a4291f3a79ed1bf82b06e4e234b65ee2607cd2b Mon Sep 17 00:00:00 2001 From: Gerald Monaco Date: Sun, 9 Feb 2020 20:37:43 +0000 Subject: [PATCH 1/3] Clean up async in next-server --- .../next/next-server/server/next-server.ts | 165 +++++++++--------- 1 file changed, 81 insertions(+), 84 deletions(-) diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index 8d43b76ceb884cd..ae5bac789c034e3 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -227,7 +227,7 @@ export default class Server { console.error(err) } - private handleRequest( + private async handleRequest( req: IncomingMessage, res: ServerResponse, parsedUrl?: UrlWithParsedQuery @@ -254,11 +254,13 @@ export default class Server { } res.statusCode = 200 - return this.run(req, res, parsedUrl).catch(err => { + try { + return await this.run(req, res, parsedUrl) + } catch (err) { this.logError(err) res.statusCode = 500 res.end('Internal Server Error') - }) + } } public getRequestHandler() { @@ -1037,33 +1039,28 @@ export default class Server { this.__sendPayload(res, html, 'text/html; charset=utf-8') } - return doRender(ssgCacheKey, []).then( - async ({ isOrigin, value: { html, pageData, sprRevalidate } }) => { - // Respond to the request if a payload wasn't sent above (from cache) - if (!isResSent(res)) { - this.__sendPayload( - res, - isDataReq ? JSON.stringify(pageData) : html, - isDataReq ? 'application/json' : 'text/html; charset=utf-8', - sprRevalidate - ) - } + const { + isOrigin, + value: { html, pageData, sprRevalidate }, + } = await doRender(ssgCacheKey, []) + if (!isResSent(res)) { + this.__sendPayload( + res, + isDataReq ? JSON.stringify(pageData) : html, + isDataReq ? 'application/json' : 'text/html; charset=utf-8', + sprRevalidate + ) + } - // Update the SPR cache if the head request - if (isOrigin) { - await setSprCache( - ssgCacheKey, - { html: html!, pageData }, - sprRevalidate - ) - } + // Update the SPR cache if the head request + if (isOrigin) { + await setSprCache(ssgCacheKey, { html: html!, pageData }, sprRevalidate) + } - return null - } - ) + return null } - public renderToHTML( + public async renderToHTML( req: IncomingMessage, res: ServerResponse, pathname: string, @@ -1076,69 +1073,60 @@ export default class Server { hasAmp?: boolean } = {} ): Promise { - return this.findPageComponents(pathname, query) - .then( - result => { - return this.renderToHTMLWithComponents( + try { + try { + const result = await this.findPageComponents(pathname, query) + return await this.renderToHTMLWithComponents( + req, + res, + pathname, + getQueryForComponents(result, query), + result, + { ...this.renderOpts, amphtml, hasAmp } + ) + } catch (err) { + if (err.code !== 'ENOENT' || !this.dynamicRoutes) { + throw err + } + + for (const dynamicRoute of this.dynamicRoutes) { + const params = dynamicRoute.match(pathname) + if (!params) { + continue + } + + const result = await this.findPageComponents(dynamicRoute.page, query) + return await this.renderToHTMLWithComponents( req, res, - pathname, - result.unstable_getStaticProps - ? { _nextDataReq: query._nextDataReq } - : query, + dynamicRoute.page, + // only add params for SPR enabled pages + { + ...getQueryForComponents(result, query), + ...params, + }, result, - { ...this.renderOpts, amphtml, hasAmp } - ) - }, - err => { - if (err.code !== 'ENOENT' || !this.dynamicRoutes) { - return Promise.reject(err) - } - - for (const dynamicRoute of this.dynamicRoutes) { - const params = dynamicRoute.match(pathname) - if (!params) { - continue + { + ...this.renderOpts, + params, + amphtml, + hasAmp, } - - return this.findPageComponents(dynamicRoute.page, query).then( - result => { - return this.renderToHTMLWithComponents( - req, - res, - dynamicRoute.page, - // only add params for SPR enabled pages - { - ...(result.unstable_getStaticProps - ? { _nextDataReq: query._nextDataReq } - : query), - ...params, - }, - result, - { - ...this.renderOpts, - params, - amphtml, - hasAmp, - } - ) - } - ) - } - - return Promise.reject(err) - } - ) - .catch(err => { - if (err && err.code === 'ENOENT') { - res.statusCode = 404 - return this.renderErrorToHTML(null, req, res, pathname, query) - } else { - this.logError(err) - res.statusCode = 500 - return this.renderErrorToHTML(err, req, res, pathname, query) + ) } - }) + + throw err + } + } catch (err) { + if (err && err.code === 'ENOENT') { + res.statusCode = 404 + return await this.renderErrorToHTML(null, req, res, pathname, query) + } else { + this.logError(err) + res.statusCode = 500 + return await this.renderErrorToHTML(err, req, res, pathname, query) + } + } } public async renderError( @@ -1300,3 +1288,12 @@ export default class Server { return isTargetLikeServerless(this.nextConfig.target) } } + +function getQueryForComponents( + components: LoadComponentsReturnType, + query: ParsedUrlQuery +) { + return components.unstable_getStaticProps + ? { _nextDataReq: query._nextDataReq } + : query +} From fd1fdfc8dd454067fdfafa95c0b5984596f08445 Mon Sep 17 00:00:00 2001 From: Gerald Monaco Date: Sun, 9 Feb 2020 21:09:46 +0000 Subject: [PATCH 2/3] fix case when err is null --- packages/next/next-server/server/next-server.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index ae5bac789c034e3..6b8e9acb714b8a2 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -1085,7 +1085,7 @@ export default class Server { { ...this.renderOpts, amphtml, hasAmp } ) } catch (err) { - if (err.code !== 'ENOENT' || !this.dynamicRoutes) { + if (err?.code !== 'ENOENT' || !this.dynamicRoutes) { throw err } From 29341fa710f91e101591075953b9b393fef1fd20 Mon Sep 17 00:00:00 2001 From: Gerald Monaco Date: Sun, 9 Feb 2020 21:45:20 +0000 Subject: [PATCH 3/3] Refactor how components are loaded --- .../next/next-server/server/next-server.ts | 164 ++++++++---------- 1 file changed, 74 insertions(+), 90 deletions(-) diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index 6b8e9acb714b8a2..8f31828d2a1c3bd 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -69,6 +69,11 @@ type Middleware = ( next: (err?: Error) => void ) => void +type FindComponentsResult = { + components: LoadComponentsReturnType + query: ParsedUrlQuery +} + export type ServerConstructor = { /** * Where the Next project is located - @default '.' @@ -814,28 +819,36 @@ export default class Server { private async findPageComponents( pathname: string, - query: ParsedUrlQuery = {} - ) { - const serverless = !this.renderOpts.dev && this._isLikeServerless - // try serving a static AMP version first - if (query.amp) { + query: ParsedUrlQuery = {}, + params: Params | null = null + ): Promise { + const paths = [ + // try serving a static AMP version first + query.amp ? normalizePagePath(pathname) + '.amp' : null, + pathname, + ].filter(Boolean) + for (const pagePath of paths) { try { - return await loadComponents( + const components = await loadComponents( this.distDir, this.buildId, - normalizePagePath(pathname) + '.amp', - serverless + pagePath!, + !this.renderOpts.dev && this._isLikeServerless ) + return { + components, + query: { + ...(components.unstable_getStaticProps + ? { _nextDataReq: query._nextDataReq } + : query), + ...(params || {}), + }, + } } catch (err) { if (err.code !== 'ENOENT') throw err } } - return await loadComponents( - this.distDir, - this.buildId, - pathname, - serverless - ) + return null } private __sendPayload( @@ -881,8 +894,7 @@ export default class Server { req: IncomingMessage, res: ServerResponse, pathname: string, - query: ParsedUrlQuery = {}, - result: LoadComponentsReturnType, + { components, query }: FindComponentsResult, opts: any ): Promise { // we need to ensure the status code if /404 is visited directly @@ -891,16 +903,16 @@ export default class Server { } // handle static page - if (typeof result.Component === 'string') { - return result.Component + if (typeof components.Component === 'string') { + return components.Component } // check request state const isLikeServerless = - typeof result.Component === 'object' && - typeof (result.Component as any).renderReqToHTML === 'function' - const isSSG = !!result.unstable_getStaticProps - const isServerProps = !!result.unstable_getServerProps + typeof components.Component === 'object' && + typeof (components.Component as any).renderReqToHTML === 'function' + const isSSG = !!components.unstable_getStaticProps + const isServerProps = !!components.unstable_getServerProps // Toggle whether or not this is a Data request const isDataReq = query._nextDataReq @@ -922,7 +934,7 @@ export default class Server { // handle serverless if (isLikeServerless) { if (isDataReq) { - const renderResult = await (result.Component as any).renderReqToHTML( + const renderResult = await (components.Component as any).renderReqToHTML( req, res, true @@ -937,12 +949,12 @@ export default class Server { return null } this.prepareServerlessUrl(req, query) - return (result.Component as any).renderReqToHTML(req, res) + return (components.Component as any).renderReqToHTML(req, res) } if (isDataReq && isServerProps) { const props = await renderToHTML(req, res, pathname, query, { - ...result, + ...components, ...opts, isDataReq, }) @@ -951,7 +963,7 @@ export default class Server { } return renderToHTML(req, res, pathname, query, { - ...result, + ...components, ...opts, }) } @@ -993,7 +1005,7 @@ export default class Server { let renderResult // handle serverless if (isLikeServerless) { - renderResult = await (result.Component as any).renderReqToHTML( + renderResult = await (components.Component as any).renderReqToHTML( req, res, true @@ -1004,7 +1016,7 @@ export default class Server { sprRevalidate = renderResult.renderOpts.revalidate } else { const renderOpts = { - ...result, + ...components, ...opts, } renderResult = await renderToHTML(req, res, pathname, query, renderOpts) @@ -1027,10 +1039,10 @@ export default class Server { query.__nextFallback = 'true' if (isLikeServerless) { this.prepareServerlessUrl(req, query) - html = await (result.Component as any).renderReqToHTML(req, res) + html = await (components.Component as any).renderReqToHTML(req, res) } else { html = (await renderToHTML(req, res, pathname, query, { - ...result, + ...components, ...opts, })) as string } @@ -1074,59 +1086,53 @@ export default class Server { } = {} ): Promise { try { - try { - const result = await this.findPageComponents(pathname, query) + const result = await this.findPageComponents(pathname, query) + if (result) { return await this.renderToHTMLWithComponents( req, res, pathname, - getQueryForComponents(result, query), result, { ...this.renderOpts, amphtml, hasAmp } ) - } catch (err) { - if (err?.code !== 'ENOENT' || !this.dynamicRoutes) { - throw err - } + } + if (this.dynamicRoutes) { for (const dynamicRoute of this.dynamicRoutes) { const params = dynamicRoute.match(pathname) if (!params) { continue } - const result = await this.findPageComponents(dynamicRoute.page, query) - return await this.renderToHTMLWithComponents( - req, - res, + const result = await this.findPageComponents( dynamicRoute.page, - // only add params for SPR enabled pages - { - ...getQueryForComponents(result, query), - ...params, - }, - result, - { - ...this.renderOpts, - params, - amphtml, - hasAmp, - } + query, + params ) + if (result) { + return await this.renderToHTMLWithComponents( + req, + res, + dynamicRoute.page, + result, + { + ...this.renderOpts, + params, + amphtml, + hasAmp, + } + ) + } } - - throw err } } catch (err) { - if (err && err.code === 'ENOENT') { - res.statusCode = 404 - return await this.renderErrorToHTML(null, req, res, pathname, query) - } else { - this.logError(err) - res.statusCode = 500 - return await this.renderErrorToHTML(err, req, res, pathname, query) - } + this.logError(err) + res.statusCode = 500 + return await this.renderErrorToHTML(err, req, res, pathname, query) } + + res.statusCode = 404 + return await this.renderErrorToHTML(null, req, res, pathname, query) } public async renderError( @@ -1154,7 +1160,7 @@ export default class Server { _pathname: string, query: ParsedUrlQuery = {} ) { - let result: null | LoadComponentsReturnType = null + let result: null | FindComponentsResult = null const { static404, pages404 } = this.nextConfig.experimental const is404 = res.statusCode === 404 @@ -1163,26 +1169,14 @@ export default class Server { // use static 404 page if available and is 404 response if (is404) { if (static404) { - try { - result = await this.findPageComponents('/_errors/404') - } catch (err) { - if (err.code !== 'ENOENT') { - throw err - } - } + result = await this.findPageComponents('/_errors/404') } // use 404 if /_errors/404 isn't available which occurs // during development and when _app has getInitialProps if (!result && pages404) { - try { - result = await this.findPageComponents('/404') - using404Page = true - } catch (err) { - if (err.code !== 'ENOENT') { - throw err - } - } + result = await this.findPageComponents('/404') + using404Page = result !== null } } @@ -1196,8 +1190,7 @@ export default class Server { req, res, using404Page ? '/404' : '/_error', - query, - result, + result!, { ...this.renderOpts, err, @@ -1288,12 +1281,3 @@ export default class Server { return isTargetLikeServerless(this.nextConfig.target) } } - -function getQueryForComponents( - components: LoadComponentsReturnType, - query: ParsedUrlQuery -) { - return components.unstable_getStaticProps - ? { _nextDataReq: query._nextDataReq } - : query -}