From 72d2f751e9886c0c7e3b0025cb96bd3d634cf734 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Mon, 9 Dec 2019 10:04:17 -0600 Subject: [PATCH 1/4] De-dupe pagesManifest --- .../next/next-server/server/next-server.ts | 50 +++++++++++-------- packages/next/server/next-dev-server.ts | 17 ++++--- 2 files changed, 38 insertions(+), 29 deletions(-) diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index 04ca6adb915fa..3a7942afb93b8 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -84,7 +84,7 @@ export default class Server { pagesDir?: string publicDir: string hasStaticDir: boolean - pagesManifest: string + pagesManifest?: { [name: string]: string } buildId: string renderOpts: { poweredByHeader: boolean @@ -122,13 +122,6 @@ export default class Server { this.distDir = join(this.dir, this.nextConfig.distDir) this.publicDir = join(this.dir, CLIENT_PUBLIC_FILES_PATH) this.hasStaticDir = fs.existsSync(join(this.dir, 'static')) - this.pagesManifest = join( - this.distDir, - this.nextConfig.target === 'server' - ? SERVER_DIRECTORY - : SERVERLESS_DIRECTORY, - PAGES_MANIFEST - ) // Only serverRuntimeConfig needs the default // publicRuntimeConfig gets it's default in client/index.js @@ -172,19 +165,26 @@ export default class Server { }) } + const serverBuildPath = join( + this.distDir, + this._isLikeServerless ? SERVERLESS_DIRECTORY : SERVER_DIRECTORY + ) + const pagesManifestPath = join(serverBuildPath, PAGES_MANIFEST) + + if (!dev) { + this.pagesManifest = require(pagesManifestPath) + } + this.router = new Router(this.generateRoutes()) this.setAssetPrefix(assetPrefix) // call init-server middleware, this is also handled // individually in serverless bundles when deployed if (!dev && this.nextConfig.experimental.plugins) { - const serverPath = join( - this.distDir, - this._isLikeServerless ? 'serverless' : 'server' - ) - const initServer = require(join(serverPath, 'init-server.js')).default + const initServer = require(join(serverBuildPath, 'init-server.js')) + .default this.onErrorMiddleware = require(join( - serverPath, + serverBuildPath, 'on-error-server.js' )).default initServer() @@ -503,6 +503,15 @@ export default class Server { return routes } + protected async hasPage(pathname: string): Promise { + return !!getPagePath( + pathname, + this.distDir, + this._isLikeServerless, + this.renderOpts.dev + ) + } + protected async _beforeCatchAllRender( _req: IncomingMessage, _res: ServerResponse, @@ -580,16 +589,12 @@ export default class Server { protected generatePublicRoutes(): Route[] { const routes: Route[] = [] const publicFiles = recursiveReadDirSync(this.publicDir) - const serverBuildPath = join( - this.distDir, - this._isLikeServerless ? SERVERLESS_DIRECTORY : SERVER_DIRECTORY - ) - const pagesManifest = require(join(serverBuildPath, PAGES_MANIFEST)) publicFiles.forEach(path => { const unixPath = path.replace(/\\/g, '/') // Only include public files that will not replace a page path - if (!pagesManifest[unixPath]) { + // this should not occur now that we check this during build + if (!this.pagesManifest![unixPath]) { routes.push({ match: route(unixPath), type: 'route', @@ -609,8 +614,9 @@ export default class Server { } protected getDynamicRoutes() { - const manifest = require(this.pagesManifest) - const dynamicRoutedPages = Object.keys(manifest).filter(isDynamicRoute) + const dynamicRoutedPages = Object.keys(this.pagesManifest!).filter( + isDynamicRoute + ) return getSortedRoutes(dynamicRoutedPages).map(page => ({ page, match: getRouteMatcher(getRouteRegex(page)), diff --git a/packages/next/server/next-dev-server.ts b/packages/next/server/next-dev-server.ts index 94390309afaf6..7a002680d5e1a 100644 --- a/packages/next/server/next-dev-server.ts +++ b/packages/next/server/next-dev-server.ts @@ -245,6 +245,15 @@ export default class DevServer extends Server { } } + protected async hasPage(pathname: string): Promise { + const pageFile = await findPageFile( + this.pagesDir!, + normalizePagePath(pathname), + this.nextConfig.pageExtensions + ) + return !!pageFile + } + protected async _beforeCatchAllRender( req: IncomingMessage, res: ServerResponse, @@ -256,13 +265,7 @@ export default class DevServer extends Server { // check for a public file, throwing error if there's a // conflicting page if (await this.hasPublicFile(pathname!)) { - const pageFile = await findPageFile( - this.pagesDir!, - normalizePagePath(pathname!), - this.nextConfig.pageExtensions - ) - - if (pageFile) { + if (await this.hasPage(pathname!)) { const err = new Error( `A conflicting public file and page file was found for path ${pathname} https://err.sh/zeit/next.js/conflicting-public-file-page` ) From a3cfe2425c07ea579a2eced5f0737f3e5b337057 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Mon, 9 Dec 2019 10:10:39 -0600 Subject: [PATCH 2/4] Update handleApiRequest a bit --- .../next/next-server/server/next-server.ts | 23 +++++++++---------- packages/next/server/next-dev-server.ts | 3 +-- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index 3a7942afb93b8..3370805a9c1cd 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -533,32 +533,31 @@ export default class Server { pathname: string ) { let params: Params | boolean = false - let resolverFunction: any + let apiPagePath: string | null = null try { - resolverFunction = await this.resolveApiRequest(pathname) - } catch (err) {} + apiPagePath = await this.resolveApiRequest(pathname) + } catch (err) { + // if an error other than 404 occurred then rethrow + if (err.code !== 'ENOENT') throw err + } - if ( - this.dynamicRoutes && - this.dynamicRoutes.length > 0 && - !resolverFunction - ) { + if (this.dynamicRoutes && this.dynamicRoutes.length > 0 && !apiPagePath) { for (const dynamicRoute of this.dynamicRoutes) { params = dynamicRoute.match(pathname) if (params) { - resolverFunction = await this.resolveApiRequest(dynamicRoute.page) + apiPagePath = await this.resolveApiRequest(dynamicRoute.page) break } } } - if (!resolverFunction) { + if (!apiPagePath) { return this.render404(req, res) } if (!this.renderOpts.dev && this._isLikeServerless) { - const mod = require(resolverFunction) + const mod = require(apiPagePath) if (typeof mod.default === 'function') { return mod.default(req, res) } @@ -568,7 +567,7 @@ export default class Server { req, res, params, - resolverFunction ? require(resolverFunction) : undefined, + require(apiPagePath), this.onErrorMiddleware ) } diff --git a/packages/next/server/next-dev-server.ts b/packages/next/server/next-dev-server.ts index 7a002680d5e1a..0dcd82e05316d 100644 --- a/packages/next/server/next-dev-server.ts +++ b/packages/next/server/next-dev-server.ts @@ -394,8 +394,7 @@ export default class DevServer extends Server { return null } } - const resolvedPath = await super.resolveApiRequest(pathname) - return resolvedPath + return super.resolveApiRequest(pathname) } async renderToHTML( From a2d3d888564d30fd7023ab6d8c63f0a733bc9cdd Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Mon, 9 Dec 2019 12:53:36 -0600 Subject: [PATCH 3/4] Simplify handleApiRequest a bit --- packages/next/next-server/server/api-utils.ts | 27 ++++--- .../next/next-server/server/next-server.ts | 78 +++++++++---------- packages/next/server/next-dev-server.ts | 16 +--- 3 files changed, 53 insertions(+), 68 deletions(-) diff --git a/packages/next/next-server/server/api-utils.ts b/packages/next/next-server/server/api-utils.ts index b7664b7f33bf1..2de7ab56cd5b4 100644 --- a/packages/next/next-server/server/api-utils.ts +++ b/packages/next/next-server/server/api-utils.ts @@ -1,4 +1,4 @@ -import { IncomingMessage } from 'http' +import { IncomingMessage, ServerResponse } from 'http' import { NextApiResponse, NextApiRequest } from '../lib/utils' import { Stream } from 'stream' import getRawBody from 'raw-body' @@ -11,12 +11,15 @@ export type NextApiRequestCookies = { [key: string]: string } export type NextApiRequestQuery = { [key: string]: string | string[] } export async function apiResolver( - req: NextApiRequest, - res: NextApiResponse, + req: IncomingMessage, + res: ServerResponse, params: any, resolverModule: any, onError?: ({ err }: { err: any }) => Promise ) { + const apiReq = req as NextApiRequest + const apiRes = res as NextApiResponse + try { let config: PageConfig = {} let bodyParser = true @@ -33,32 +36,32 @@ export async function apiResolver( } } // Parsing of cookies - setLazyProp({ req }, 'cookies', getCookieParser(req)) + setLazyProp({ req: apiReq }, 'cookies', getCookieParser(req)) // Parsing query string - setLazyProp({ req, params }, 'query', getQueryParser(req)) + setLazyProp({ req: apiReq, params }, 'query', getQueryParser(req)) // // Parsing of body if (bodyParser) { - req.body = await parseBody( - req, + apiReq.body = await parseBody( + apiReq, config.api && config.api.bodyParser && config.api.bodyParser.sizeLimit ? config.api.bodyParser.sizeLimit : '1mb' ) } - res.status = statusCode => sendStatusCode(res, statusCode) - res.send = data => sendData(res, data) - res.json = data => sendJson(res, data) + apiRes.status = statusCode => sendStatusCode(apiRes, statusCode) + apiRes.send = data => sendData(apiRes, data) + apiRes.json = data => sendJson(apiRes, data) const resolver = interopDefault(resolverModule) resolver(req, res) } catch (err) { if (err instanceof ApiError) { - sendError(res, err.statusCode, err.message) + sendError(apiRes, err.statusCode, err.message) } else { console.error(err) if (onError) await onError({ err }) - sendError(res, 500, 'Internal Server Error') + sendError(apiRes, 500, 'Internal Server Error') } } } diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index 3370805a9c1cd..b5b44ae4694af 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -84,6 +84,7 @@ export default class Server { pagesDir?: string publicDir: string hasStaticDir: boolean + serverBuildDir: string pagesManifest?: { [name: string]: string } buildId: string renderOpts: { @@ -165,11 +166,11 @@ export default class Server { }) } - const serverBuildPath = join( + this.serverBuildDir = join( this.distDir, this._isLikeServerless ? SERVERLESS_DIRECTORY : SERVER_DIRECTORY ) - const pagesManifestPath = join(serverBuildPath, PAGES_MANIFEST) + const pagesManifestPath = join(this.serverBuildDir, PAGES_MANIFEST) if (!dev) { this.pagesManifest = require(pagesManifestPath) @@ -181,10 +182,10 @@ export default class Server { // call init-server middleware, this is also handled // individually in serverless bundles when deployed if (!dev && this.nextConfig.experimental.plugins) { - const initServer = require(join(serverBuildPath, 'init-server.js')) + const initServer = require(join(this.serverBuildDir, 'init-server.js')) .default this.onErrorMiddleware = require(join( - serverBuildPath, + this.serverBuildDir, 'on-error-server.js' )).default initServer() @@ -503,8 +504,8 @@ export default class Server { return routes } - protected async hasPage(pathname: string): Promise { - return !!getPagePath( + protected async getPagePath(pathname: string) { + return getPagePath( pathname, this.distDir, this._isLikeServerless, @@ -512,6 +513,15 @@ export default class Server { ) } + protected async hasPage(pathname: string): Promise { + let found = false + try { + found = !!(await this.getPagePath(pathname)) + } catch (_) {} + + return found + } + protected async _beforeCatchAllRender( _req: IncomingMessage, _res: ServerResponse, @@ -521,6 +531,9 @@ export default class Server { return false } + // Used to build API page in development + protected async ensureApiPage(pathname: string) {} + /** * Resolves `API` request, in development builds on demand * @param req http request @@ -528,61 +541,42 @@ export default class Server { * @param pathname path of request */ private async handleApiRequest( - req: NextApiRequest, - res: NextApiResponse, + req: IncomingMessage, + res: ServerResponse, pathname: string ) { + let page = pathname let params: Params | boolean = false - let apiPagePath: string | null = null - - try { - apiPagePath = await this.resolveApiRequest(pathname) - } catch (err) { - // if an error other than 404 occurred then rethrow - if (err.code !== 'ENOENT') throw err - } + let pageFound = await this.hasPage(page) - if (this.dynamicRoutes && this.dynamicRoutes.length > 0 && !apiPagePath) { + if (!pageFound && this.dynamicRoutes) { for (const dynamicRoute of this.dynamicRoutes) { params = dynamicRoute.match(pathname) if (params) { - apiPagePath = await this.resolveApiRequest(dynamicRoute.page) + page = dynamicRoute.page + pageFound = true break } } } - if (!apiPagePath) { + if (!pageFound) { return this.render404(req, res) } + // Make sure the page is built before getting the path + // or else it won't be in the manifest yet + await this.ensureApiPage(page) + + const builtPagePath = await this.getPagePath(page) + const pageModule = require(builtPagePath) if (!this.renderOpts.dev && this._isLikeServerless) { - const mod = require(apiPagePath) - if (typeof mod.default === 'function') { - return mod.default(req, res) + if (typeof pageModule.default === 'function') { + return pageModule.default(req, res) } } - await apiResolver( - req, - res, - params, - require(apiPagePath), - this.onErrorMiddleware - ) - } - - /** - * Resolves path to resolver function - * @param pathname path of request - */ - protected async resolveApiRequest(pathname: string): Promise { - return getPagePath( - pathname, - this.distDir, - this._isLikeServerless, - this.renderOpts.dev - ) + await apiResolver(req, res, params, pageModule, this.onErrorMiddleware) } protected generatePublicRoutes(): Route[] { diff --git a/packages/next/server/next-dev-server.ts b/packages/next/server/next-dev-server.ts index 0dcd82e05316d..57ae55dd178b3 100644 --- a/packages/next/server/next-dev-server.ts +++ b/packages/next/server/next-dev-server.ts @@ -381,20 +381,8 @@ export default class DevServer extends Server { return !snippet.includes('data-amp-development-mode-only') } - /** - * Check if resolver function is build or request new build for this function - * @param {string} pathname - */ - protected async resolveApiRequest(pathname: string): Promise { - try { - await this.hotReloader!.ensurePage(pathname) - } catch (err) { - // API route dosn't exist => return 404 - if (err.code === 'ENOENT') { - return null - } - } - return super.resolveApiRequest(pathname) + protected async ensureApiPage(pathname: string) { + return this.hotReloader!.ensurePage(pathname) } async renderToHTML( From fc7245c2bfdee7197b33a92e33775bfece205348 Mon Sep 17 00:00:00 2001 From: Joe Haddad Date: Tue, 10 Dec 2019 10:00:51 -0500 Subject: [PATCH 4/4] Update packages/next/next-server/server/next-server.ts --- 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 b5b44ae4694af..4ba0a0e060853 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -504,7 +504,7 @@ export default class Server { return routes } - protected async getPagePath(pathname: string) { + private async getPagePath(pathname: string) { return getPagePath( pathname, this.distDir,