From 3f94f33ba2111b7a757ef39a5b0c8562b8d16682 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Thu, 11 Feb 2021 03:31:49 -0600 Subject: [PATCH 1/2] Ensure error is passed up in minimal mode (#22030) This ensures we pass the error up to the top-level when in minimal mode --- .../next/next-server/server/next-server.ts | 28 ++++++++++++----- .../required-server-files/pages/errors/gip.js | 14 +++++++++ .../pages/errors/gsp/[post].js | 28 +++++++++++++++++ .../pages/errors/gssp.js | 16 ++++++++++ .../required-server-files/test/index.test.js | 31 ++++++++++++++++++- 5 files changed, 109 insertions(+), 8 deletions(-) create mode 100644 test/integration/required-server-files/pages/errors/gip.js create mode 100644 test/integration/required-server-files/pages/errors/gsp/[post].js create mode 100644 test/integration/required-server-files/pages/errors/gssp.js diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index 7d0325545fe06..78964da74458e 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -184,7 +184,7 @@ export default class Server { this.nextConfig = loadConfig(phase, this.dir, conf) 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.hasStaticDir = !minimalMode && fs.existsSync(join(this.dir, 'static')) // Only serverRuntimeConfig needs the default // publicRuntimeConfig gets it's default in client/index.js @@ -567,6 +567,9 @@ export default class Server { try { return await this.run(req, res, parsedUrl) } catch (err) { + if (this.minimalMode) { + throw err + } this.logError(err) res.statusCode = 500 res.end('Internal Server Error') @@ -1835,14 +1838,19 @@ export default class Server { } } } catch (err) { - this.logError(err) - if (err && err.code === 'DECODE_FAILED') { + this.logError(err) res.statusCode = 400 return await this.renderErrorToHTML(err, req, res, pathname, query) } res.statusCode = 500 - return await this.renderErrorToHTML(err, req, res, pathname, query) + const html = await this.renderErrorToHTML(err, req, res, pathname, query) + + if (this.minimalMode) { + throw err + } + this.logError(err) + return html } res.statusCode = 404 return await this.renderErrorToHTML(null, req, res, pathname, query) @@ -1863,6 +1871,10 @@ export default class Server { ) } const html = await this.renderErrorToHTML(err, req, res, pathname, query) + + if (this.minimalMode && res.statusCode === 500) { + throw err + } if (html === null) { return } @@ -2007,9 +2019,11 @@ export default class Server { } let nextFilesStatic: string[] = [] - nextFilesStatic = recursiveReadDirSync( - join(this.distDir, 'static') - ).map((f) => join('.', relative(this.dir, this.distDir), 'static', f)) + nextFilesStatic = !this.minimalMode + ? recursiveReadDirSync(join(this.distDir, 'static')).map((f) => + join('.', relative(this.dir, this.distDir), 'static', f) + ) + : [] return (this._validFilesystemPathSet = new Set([ ...nextFilesStatic, diff --git a/test/integration/required-server-files/pages/errors/gip.js b/test/integration/required-server-files/pages/errors/gip.js new file mode 100644 index 0000000000000..c673c7de83b48 --- /dev/null +++ b/test/integration/required-server-files/pages/errors/gip.js @@ -0,0 +1,14 @@ +function Page(props) { + return

here comes an error

+} + +Page.getInitialProps = ({ query }) => { + if (query.crash) { + throw new Error('gip hit an oops') + } + return { + hello: 'world', + } +} + +export default Page diff --git a/test/integration/required-server-files/pages/errors/gsp/[post].js b/test/integration/required-server-files/pages/errors/gsp/[post].js new file mode 100644 index 0000000000000..b696ff821aa7b --- /dev/null +++ b/test/integration/required-server-files/pages/errors/gsp/[post].js @@ -0,0 +1,28 @@ +import { useRouter } from 'next/router' + +function Page(props) { + if (useRouter().isFallback) { + return

loading...

+ } + return

here comes an error

+} + +export const getStaticPaths = () => { + return { + paths: [], + fallback: true, + } +} + +export const getStaticProps = ({ params }) => { + if (params.post === 'crash') { + throw new Error('gsp hit an oops') + } + return { + props: { + hello: 'world', + }, + } +} + +export default Page diff --git a/test/integration/required-server-files/pages/errors/gssp.js b/test/integration/required-server-files/pages/errors/gssp.js new file mode 100644 index 0000000000000..4e1adfa9575fd --- /dev/null +++ b/test/integration/required-server-files/pages/errors/gssp.js @@ -0,0 +1,16 @@ +function Page(props) { + return

here comes an error

+} + +export const getServerSideProps = ({ query }) => { + if (query.crash) { + throw new Error('gssp hit an oops') + } + return { + props: { + hello: 'world', + }, + } +} + +export default Page diff --git a/test/integration/required-server-files/test/index.test.js b/test/integration/required-server-files/test/index.test.js index 97a1fc7e12d8f..8ae560b690081 100644 --- a/test/integration/required-server-files/test/index.test.js +++ b/test/integration/required-server-files/test/index.test.js @@ -20,6 +20,7 @@ let nextApp let appPort let buildId let requiredFilesManifest +let errors = [] describe('Required Server Files', () => { beforeAll(async () => { @@ -58,7 +59,8 @@ describe('Required Server Files', () => { try { await nextApp.getRequestHandler()(req, res) } catch (err) { - console.error(err) + console.error('top-level', err) + errors.push(err) res.statusCode = 500 res.end('error') } @@ -419,4 +421,31 @@ describe('Required Server Files', () => { path: ['hello', 'world'], }) }) + + it('should bubble error correctly for gip page', async () => { + errors = [] + const res = await fetchViaHTTP(appPort, '/errors/gip', { crash: '1' }) + expect(res.status).toBe(500) + expect(await res.text()).toBe('error') + expect(errors.length).toBe(1) + expect(errors[0].message).toContain('gip hit an oops') + }) + + it('should bubble error correctly for gssp page', async () => { + errors = [] + const res = await fetchViaHTTP(appPort, '/errors/gssp', { crash: '1' }) + expect(res.status).toBe(500) + expect(await res.text()).toBe('error') + expect(errors.length).toBe(1) + expect(errors[0].message).toContain('gssp hit an oops') + }) + + it('should bubble error correctly for gsp page', async () => { + errors = [] + const res = await fetchViaHTTP(appPort, '/errors/gsp/crash') + expect(res.status).toBe(500) + expect(await res.text()).toBe('error') + expect(errors.length).toBe(1) + expect(errors[0].message).toContain('gsp hit an oops') + }) }) From 5febe218a622022fb375ef5101f1848d9c3120cc Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Thu, 11 Feb 2021 03:55:56 -0600 Subject: [PATCH 2/2] Add nccing AMP optimizer (#21980) This adds ncc'ing the AMP optimizer package to speed up install times and cache the runtime. Closes: https://github.com/vercel/next.js/issues/20404 --- packages/next/build/index.ts | 6 +++++- packages/next/build/webpack-config.ts | 2 +- .../next/next-server/server/optimize-amp.ts | 2 +- packages/next/package.json | 2 +- packages/next/taskfile-ncc.js | 10 +++++++--- packages/next/taskfile.js | 19 +++++++++++++++++++ test/.stats-app/pages/amp.js | 9 +++++++++ 7 files changed, 43 insertions(+), 7 deletions(-) create mode 100644 test/.stats-app/pages/amp.js diff --git a/packages/next/build/index.ts b/packages/next/build/index.ts index 0ca466995703f..6a7e7d62d4d26 100644 --- a/packages/next/build/index.ts +++ b/packages/next/build/index.ts @@ -787,7 +787,11 @@ export default async function build( path.relative( dir, path.join( - path.dirname(require.resolve('@ampproject/toolbox-optimizer')), + path.dirname( + require.resolve( + 'next/dist/compiled/@ampproject/toolbox-optimizer' + ) + ), '**/*' ) ) diff --git a/packages/next/build/webpack-config.ts b/packages/next/build/webpack-config.ts index ff3698cd28b7a..2a754fdcd67f3 100644 --- a/packages/next/build/webpack-config.ts +++ b/packages/next/build/webpack-config.ts @@ -763,7 +763,7 @@ export default async function getBaseWebpackConfig( : [ // When the 'serverless' target is used all node_modules will be compiled into the output bundles // So that the 'serverless' bundles have 0 runtime dependencies - '@ampproject/toolbox-optimizer', // except this one + 'next/dist/compiled/@ampproject/toolbox-optimizer', // except this one // Mark this as external if not enabled so it doesn't cause a // webpack error from being missing diff --git a/packages/next/next-server/server/optimize-amp.ts b/packages/next/next-server/server/optimize-amp.ts index 6e7d3345c9fce..376191136f15f 100644 --- a/packages/next/next-server/server/optimize-amp.ts +++ b/packages/next/next-server/server/optimize-amp.ts @@ -4,7 +4,7 @@ export default async function optimize( ): Promise { let AmpOptimizer try { - AmpOptimizer = require('@ampproject/toolbox-optimizer') + AmpOptimizer = require('next/dist/compiled/@ampproject/toolbox-optimizer') } catch (_) { return html } diff --git a/packages/next/package.json b/packages/next/package.json index d05ed42d9259f..63a4bb51297a3 100644 --- a/packages/next/package.json +++ b/packages/next/package.json @@ -60,7 +60,6 @@ ] }, "dependencies": { - "@ampproject/toolbox-optimizer": "2.7.1-alpha.0", "@babel/runtime": "7.12.5", "@hapi/accept": "5.0.1", "@next/env": "10.0.7-canary.6", @@ -120,6 +119,7 @@ } }, "devDependencies": { + "@ampproject/toolbox-optimizer": "2.7.1-alpha.0", "@babel/code-frame": "7.12.11", "@babel/core": "7.12.10", "@babel/plugin-proposal-class-properties": "7.12.1", diff --git a/packages/next/taskfile-ncc.js b/packages/next/taskfile-ncc.js index eeb0cba2a667a..5da22a555cd49 100644 --- a/packages/next/taskfile-ncc.js +++ b/packages/next/taskfile-ncc.js @@ -19,6 +19,9 @@ module.exports = function (task) { options.externals = { ...options.externals } delete options.externals[options.packageName] } + let precompiled = options.precompiled !== false + delete options.precompiled + return ncc(join(__dirname, file.dir, file.base), { filename: file.base, minify: options.minify === false ? false : true, @@ -39,7 +42,8 @@ module.exports = function (task) { this, options.packageName, file.base, - options.bundleName + options.bundleName, + precompiled ) } @@ -51,13 +55,13 @@ module.exports = function (task) { // This function writes a minimal `package.json` file for a compiled package. // It defines `name`, `main`, `author`, and `license`. It also defines `types`. // n.b. types intended for development usage only. -function writePackageManifest(packageName, main, bundleName) { +function writePackageManifest(packageName, main, bundleName, precompiled) { const packagePath = bundleRequire.resolve(packageName + '/package.json') let { name, author, license } = require(packagePath) const compiledPackagePath = join( __dirname, - `compiled/${bundleName || packageName}` + `${!precompiled ? 'dist/' : ''}compiled/${bundleName || packageName}` ) const potentialLicensePath = join(dirname(packagePath), './LICENSE') diff --git a/packages/next/taskfile.js b/packages/next/taskfile.js index b5a50d05f1a43..f1666b114d3df 100644 --- a/packages/next/taskfile.js +++ b/packages/next/taskfile.js @@ -68,6 +68,22 @@ export async function ncc_amphtml_validator(task, opts) { .target('compiled/amphtml-validator') } // eslint-disable-next-line camelcase +externals['@ampproject/toolbox-optimizer'] = + 'next/dist/compiled/@ampproject/toolbox-optimizer' +export async function ncc_amp_optimizer(task, opts) { + await task + .source( + opts.src || + relative(__dirname, require.resolve('@ampproject/toolbox-optimizer')) + ) + .ncc({ + externals, + precompiled: false, + packageName: '@ampproject/toolbox-optimizer', + }) + .target('dist/compiled/@ampproject/toolbox-optimizer') +} +// eslint-disable-next-line camelcase externals['arg'] = 'distcompiled/arg' export async function ncc_arg(task, opts) { await task @@ -757,6 +773,9 @@ export async function compile(task, opts) { 'client', 'telemetry', 'nextserver', + // we compile this each time so that fresh runtime data is pulled + // before each publish + 'ncc_amp_optimizer', ], opts ) diff --git a/test/.stats-app/pages/amp.js b/test/.stats-app/pages/amp.js new file mode 100644 index 0000000000000..aec5e04f289ae --- /dev/null +++ b/test/.stats-app/pages/amp.js @@ -0,0 +1,9 @@ +import { useAmp } from 'next/amp' + +export const config = { + amp: 'hybrid', +} + +export default function Amp(props) { + return useAmp() ? 'AMP mode' : 'normal mode' +}