From cdc2669725246cf7ada9ef6e92a4bcd08030fa24 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Sat, 26 Jun 2021 15:40:45 +0800 Subject: [PATCH 1/5] fix: detect loop in client error page --- packages/next/client/index.tsx | 8 ++++++ packages/next/types/misc.d.ts | 2 ++ .../pages/_error.js | 12 +++++++++ .../pages/index.js | 20 ++++++++++++++ .../test/index.test.js | 26 +++++++++++++++++++ 5 files changed, 68 insertions(+) create mode 100644 test/integration/custom-error-page-exception/pages/_error.js create mode 100644 test/integration/custom-error-page-exception/pages/index.js create mode 100644 test/integration/custom-error-page-exception/test/index.test.js diff --git a/packages/next/client/index.tsx b/packages/next/client/index.tsx index 32b6558c42b88b..8ed87aa3310742 100644 --- a/packages/next/client/index.tsx +++ b/packages/next/client/index.tsx @@ -467,6 +467,14 @@ export function renderError(renderErrorProps: RenderErrorProps): Promise { return pageLoader .loadPage('/_error') .then(({ page: ErrorComponent, styleSheets }) => { + return lastAppProps?.Component === ErrorComponent + ? import('next/dist/pages/_error').then((m) => ({ + ErrorComponent: m.default as React.ComponentType<{}>, + styleSheets: [], + })) + : { ErrorComponent, styleSheets } + }) + .then(({ ErrorComponent, styleSheets }) => { // In production we do a normal render with the `ErrorComponent` as component. // If we've gotten here upon initial render, we can use the props from the server. // Otherwise, we need to call `getInitialProps` on `App` before mounting. diff --git a/packages/next/types/misc.d.ts b/packages/next/types/misc.d.ts index 261ab7cb97f62a..40823fa01a2cb5 100644 --- a/packages/next/types/misc.d.ts +++ b/packages/next/types/misc.d.ts @@ -45,6 +45,8 @@ declare module 'next/dist/compiled/arg/index.js' { export = arg } +declare module 'next/dist/pages/_error' + declare module 'next/dist/compiled/babel/code-frame' { export * from '@babel/code-frame' } diff --git a/test/integration/custom-error-page-exception/pages/_error.js b/test/integration/custom-error-page-exception/pages/_error.js new file mode 100644 index 00000000000000..1e5fccdc31d673 --- /dev/null +++ b/test/integration/custom-error-page-exception/pages/_error.js @@ -0,0 +1,12 @@ +/* eslint-disable no-unused-expressions, no-undef */ +let renderCount = 0 + +export default function Error() { + renderCount++ + + // Guard to avoid endless loop crashing the browser tab. + if (typeof window !== 'undefined' && renderCount < 3) { + throw new Error('crash') + } + return `error threw ${renderCount} times` +} diff --git a/test/integration/custom-error-page-exception/pages/index.js b/test/integration/custom-error-page-exception/pages/index.js new file mode 100644 index 00000000000000..1314e09108650c --- /dev/null +++ b/test/integration/custom-error-page-exception/pages/index.js @@ -0,0 +1,20 @@ +/* eslint-disable no-unused-expressions, no-unused-vars */ +import React from 'react' +import Link from 'next/link' + +function page() { + return ( + + Client side nav + + ) +} + +page.getInitialProps = () => { + if (typeof window !== 'undefined') { + throw new Error('Oops from Home') + } + return {} +} + +export default page diff --git a/test/integration/custom-error-page-exception/test/index.test.js b/test/integration/custom-error-page-exception/test/index.test.js new file mode 100644 index 00000000000000..c95c35ec35cda3 --- /dev/null +++ b/test/integration/custom-error-page-exception/test/index.test.js @@ -0,0 +1,26 @@ +/* eslint-env jest */ + +import { join } from 'path' +import webdriver from 'next-webdriver' +import { nextBuild, nextStart, findPort, killApp } from 'next-test-utils' + +jest.setTimeout(1000 * 60 * 1) + +const appDir = join(__dirname, '..') +const navSel = '#nav' +const errorMessage = 'Application error: a client-side exception has occurred' + +describe('Custom error page exception', () => { + it('should handle errors from _error render', async () => { + const { code } = await nextBuild(appDir) + const appPort = await findPort() + const app = await nextStart(appDir, appPort) + const browser = await webdriver(appPort, '/') + await browser.waitForElementByCss(navSel).elementByCss(navSel).click() + const text = await (await browser.elementByCss('#__next')).text() + killApp(app) + + expect(code).toBe(0) + expect(text).toMatch(errorMessage) + }) +}) From e2cc5fc5296a09bf426ea39438e0a64c5a9202eb Mon Sep 17 00:00:00 2001 From: Tobias Koppers Date: Mon, 28 Jun 2021 17:28:01 +0200 Subject: [PATCH 2/5] transpile client files as ESM --- packages/next/taskfile-babel.js | 44 ++++++++++++++++++++------------- 1 file changed, 27 insertions(+), 17 deletions(-) diff --git a/packages/next/taskfile-babel.js b/packages/next/taskfile-babel.js index 90e2d9046f17e2..f3f188075debbf 100644 --- a/packages/next/taskfile-babel.js +++ b/packages/next/taskfile-babel.js @@ -6,26 +6,27 @@ const path = require('path') // eslint-disable-next-line import/no-extraneous-dependencies const transform = require('@babel/core').transform +const clientPath = path.resolve(__dirname, 'client') + +const babelClientPresetEnvOptions = { + modules: 'commonjs', + targets: { + esmodules: true, + }, + bugfixes: true, + loose: true, + // This is handled by the Next.js webpack config that will run next/babel over the same code. + exclude: [ + 'transform-typeof-symbol', + 'transform-async-to-generator', + 'transform-spread', + ], +} + const babelClientOpts = { presets: [ '@babel/preset-typescript', - [ - '@babel/preset-env', - { - modules: 'commonjs', - targets: { - esmodules: true, - }, - bugfixes: true, - loose: true, - // This is handled by the Next.js webpack config that will run next/babel over the same code. - exclude: [ - 'transform-typeof-symbol', - 'transform-async-to-generator', - 'transform-spread', - ], - }, - ], + ['@babel/preset-env', babelClientPresetEnvOptions], ['@babel/preset-react', { useBuiltIns: true }], ], plugins: [ @@ -40,6 +41,15 @@ const babelClientOpts = { // eslint-disable-next-line import/no-extraneous-dependencies plugins: [require('@babel/plugin-proposal-numeric-separator').default], }, + { + test: (p) => p.startsWith(clientPath), + presets: [ + [ + '@babel/preset-env', + { ...babelClientPresetEnvOptions, modules: false }, + ], + ], + }, ], } From 9124d7471d1e60afd7ca04dd542d4dc0af6b87fd Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Wed, 30 Jun 2021 01:20:03 +0800 Subject: [PATCH 3/5] omit propsal-dynamic-import --- packages/next/client/index.tsx | 2 +- packages/next/taskfile-babel.js | 12 +----------- packages/next/types/misc.d.ts | 2 -- test/integration/build-output/test/index.test.js | 10 +++++----- 4 files changed, 7 insertions(+), 19 deletions(-) diff --git a/packages/next/client/index.tsx b/packages/next/client/index.tsx index 8ed87aa3310742..7da4737023657b 100644 --- a/packages/next/client/index.tsx +++ b/packages/next/client/index.tsx @@ -468,7 +468,7 @@ export function renderError(renderErrorProps: RenderErrorProps): Promise { .loadPage('/_error') .then(({ page: ErrorComponent, styleSheets }) => { return lastAppProps?.Component === ErrorComponent - ? import('next/dist/pages/_error').then((m) => ({ + ? import('../pages/_error').then((m) => ({ ErrorComponent: m.default as React.ComponentType<{}>, styleSheets: [], })) diff --git a/packages/next/taskfile-babel.js b/packages/next/taskfile-babel.js index f3f188075debbf..b699f15346dd87 100644 --- a/packages/next/taskfile-babel.js +++ b/packages/next/taskfile-babel.js @@ -6,8 +6,6 @@ const path = require('path') // eslint-disable-next-line import/no-extraneous-dependencies const transform = require('@babel/core').transform -const clientPath = path.resolve(__dirname, 'client') - const babelClientPresetEnvOptions = { modules: 'commonjs', targets: { @@ -20,6 +18,7 @@ const babelClientPresetEnvOptions = { 'transform-typeof-symbol', 'transform-async-to-generator', 'transform-spread', + 'proposal-dynamic-import', ], } @@ -41,15 +40,6 @@ const babelClientOpts = { // eslint-disable-next-line import/no-extraneous-dependencies plugins: [require('@babel/plugin-proposal-numeric-separator').default], }, - { - test: (p) => p.startsWith(clientPath), - presets: [ - [ - '@babel/preset-env', - { ...babelClientPresetEnvOptions, modules: false }, - ], - ], - }, ], } diff --git a/packages/next/types/misc.d.ts b/packages/next/types/misc.d.ts index 40823fa01a2cb5..261ab7cb97f62a 100644 --- a/packages/next/types/misc.d.ts +++ b/packages/next/types/misc.d.ts @@ -45,8 +45,6 @@ declare module 'next/dist/compiled/arg/index.js' { export = arg } -declare module 'next/dist/pages/_error' - declare module 'next/dist/compiled/babel/code-frame' { export * from '@babel/code-frame' } diff --git a/test/integration/build-output/test/index.test.js b/test/integration/build-output/test/index.test.js index ef085280f3d455..fd6d6c86b8186a 100644 --- a/test/integration/build-output/test/index.test.js +++ b/test/integration/build-output/test/index.test.js @@ -123,16 +123,16 @@ describe('Build Output', () => { ) expect(indexSize.endsWith('B')).toBe(true) - expect(parseFloat(indexFirstLoad)).toBeCloseTo(gz ? 64 : 196, 1) + expect(parseFloat(indexFirstLoad)).toBeCloseTo(gz ? 64.7 : 198, 1) expect(indexFirstLoad.endsWith('kB')).toBe(true) expect(parseFloat(err404Size)).toBeCloseTo(gz ? 3.17 : 8.51, 1) expect(err404Size.endsWith('kB')).toBe(true) - expect(parseFloat(err404FirstLoad)).toBeCloseTo(gz ? 66.9 : 205, 1) + expect(parseFloat(err404FirstLoad)).toBeCloseTo(gz ? 67.6 : 206, 1) expect(err404FirstLoad.endsWith('kB')).toBe(true) - expect(parseFloat(sharedByAll)).toBeCloseTo(gz ? 63.7 : 196, 1) + expect(parseFloat(sharedByAll)).toBeCloseTo(gz ? 64.5 : 198, 1) expect(sharedByAll.endsWith('kB')).toBe(true) const appSizeValue = _appSize.endsWith('kB') @@ -144,12 +144,12 @@ describe('Build Output', () => { const webpackSizeValue = webpackSize.endsWith('kB') ? parseFloat(webpackSize) : parseFloat(webpackSize) / 1000 - expect(webpackSizeValue).toBeCloseTo(gz ? 0.766 : 1.46, 2) + expect(webpackSizeValue).toBeCloseTo(gz ? 1.42 : 2.9, 2) expect(webpackSize.endsWith('kB') || webpackSize.endsWith(' B')).toBe( true ) - expect(parseFloat(mainSize)).toBeCloseTo(gz ? 20.1 : 62.7, 1) + expect(parseFloat(mainSize)).toBeCloseTo(gz ? 20.2 : 63, 1) expect(mainSize.endsWith('kB')).toBe(true) expect(parseFloat(frameworkSize)).toBeCloseTo(gz ? 42.0 : 130, 1) From a2c7e29a1c88bfd23b505a90e44ea353bd208684 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Wed, 30 Jun 2021 01:57:46 +0800 Subject: [PATCH 4/5] fix test --- test/integration/size-limit/test/index.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/size-limit/test/index.test.js b/test/integration/size-limit/test/index.test.js index fde2f15a6df769..e6af658ec08c3b 100644 --- a/test/integration/size-limit/test/index.test.js +++ b/test/integration/size-limit/test/index.test.js @@ -85,6 +85,6 @@ describe('Production response size', () => { const delta = responseSizesBytes / 1024 // Expected difference: < 0.5 - expect(delta).toBeCloseTo(286.8, 0) + expect(delta).toBeCloseTo(287.9, 0) }) }) From 0b47b7ac348b96c8fffde67c24a1001ca1067c30 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Wed, 30 Jun 2021 02:20:46 +0800 Subject: [PATCH 5/5] fix test --- test/integration/fallback-modules/test/index.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/fallback-modules/test/index.test.js b/test/integration/fallback-modules/test/index.test.js index 57558d4eef5e3e..4377b7e3a1bae5 100644 --- a/test/integration/fallback-modules/test/index.test.js +++ b/test/integration/fallback-modules/test/index.test.js @@ -49,7 +49,7 @@ describe('Build Output', () => { expect(indexSize.endsWith('kB')).toBe(true) expect(parseFloat(indexFirstLoad)).toBeLessThanOrEqual( - process.env.NEXT_PRIVATE_TEST_WEBPACK4_MODE ? 68 : 67.9 + process.env.NEXT_PRIVATE_TEST_WEBPACK4_MODE ? 68.6 : 67.9 ) expect(parseFloat(indexFirstLoad)).toBeGreaterThanOrEqual(60) expect(indexFirstLoad.endsWith('kB')).toBe(true)