From 1205fc538e19dc489df13b4ee476d77a4cb74fb6 Mon Sep 17 00:00:00 2001 From: Michael Date: Thu, 12 Sep 2024 09:07:48 +0800 Subject: [PATCH] feat: downgrade fatal crash error for `` component (#596) --- examples/basic/package.json | 2 +- .../basic/src/routes/fatal-link-demo/page.tsx | 17 ++++++ packages/router-react/src/Link.tsx | 48 ++++++++++++++++- test/e2e/link-without-to-props.test.ts | 53 +++++++++++++++++++ .../basic/src/routes/fatal-link-demo/page.js | 17 ++++++ 5 files changed, 134 insertions(+), 3 deletions(-) create mode 100644 examples/basic/src/routes/fatal-link-demo/page.tsx create mode 100644 test/e2e/link-without-to-props.test.ts create mode 100644 test/fixtures/basic/src/routes/fatal-link-demo/page.js diff --git a/examples/basic/package.json b/examples/basic/package.json index 4971b4ee9..1957e4743 100644 --- a/examples/basic/package.json +++ b/examples/basic/package.json @@ -4,7 +4,7 @@ "scripts": { "dev": "shuvi dev", "build": "shuvi build && npm run lint", - "start": "shuvi start", + "start": "shuvi serve", "lint": "shuvi lint" }, "dependencies": { diff --git a/examples/basic/src/routes/fatal-link-demo/page.tsx b/examples/basic/src/routes/fatal-link-demo/page.tsx new file mode 100644 index 000000000..83661dfad --- /dev/null +++ b/examples/basic/src/routes/fatal-link-demo/page.tsx @@ -0,0 +1,17 @@ +import * as React from 'react'; +import { Link } from '@shuvi/runtime'; + +// @note test purpose to trigger runtime error +const TO = undefined as unknown as string; + +export default function Page() { + return ( +
+ Demo runtime error - Link component missing required `prop.to` +
+ +
+ ); +} diff --git a/packages/router-react/src/Link.tsx b/packages/router-react/src/Link.tsx index 87107e69d..2d73a8e15 100644 --- a/packages/router-react/src/Link.tsx +++ b/packages/router-react/src/Link.tsx @@ -34,8 +34,8 @@ function isModifiedEvent(event: React.MouseEvent) { * About => <{...rest} a> * ``` */ -export const Link = React.forwardRef( - function LinkWithRef( +const BaseLink = React.forwardRef( + function BaseLinkWithRef( { onClick, replace: replaceProp = false, state, target, to, ...rest }, ref ) { @@ -75,6 +75,50 @@ export const Link = React.forwardRef( } ); +/** + * @NOTE Improve Page Stability by Handling Fatal Crashes 致命錯誤降級處理 + * + * Development Mode: + * On fatal errors, immediately show the "Internal Application Error" page. + * + * Production Mode: Downgrade fatal error + * 1. console.error without causing an immediate page crash. + * 2. Only after user clicks , page re-render + * and display the "Internal Application Error" page. + * + * @issue https://github.com/shuvijs/shuvi/pull/596 + */ +export const Link = React.forwardRef( + function LinkWithRef(props, ref) { + const invalidPropTo = typeof props.to === 'undefined'; + if (invalidPropTo) { + console.error( + `The prop 'to' is required in '', but its value is 'undefined'`, + JSON.stringify({ props }) + ); + } + + const [downgradeError, setDowngradeError] = React.useState( + process.env.NODE_ENV === 'production' + ); + + if (downgradeError && invalidPropTo) { + return ( + { + e.preventDefault(); + setDowngradeError(false); + }} + ref={ref} + /> + ); + } + + return ; + } +); + export interface LinkProps extends Omit, 'href'> { replace?: boolean; diff --git a/test/e2e/link-without-to-props.test.ts b/test/e2e/link-without-to-props.test.ts new file mode 100644 index 000000000..4880a7f00 --- /dev/null +++ b/test/e2e/link-without-to-props.test.ts @@ -0,0 +1,53 @@ +import { AppCtx, Page, devFixture, serveFixture } from '../utils'; + +let ctx: AppCtx; +let page: Page; + +jest.setTimeout(5 * 60 * 1000); + +describe('link prop.to - [dev mode]', () => { + beforeAll(async () => { + ctx = await devFixture('basic', { ssr: true }); + }); + afterAll(async () => { + await ctx.close(); + }); + afterEach(async () => { + await page.close(); + }); + + test(`immediately show the "Internal Application Error" page`, async () => { + page = await ctx.browser.page(ctx.url('/fatal-link-demo')); + expect(await page.$text('#__APP')).toContain( + `500` // 500 error + ); + expect(await page.$text('#__APP')).toContain( + `Cannot read properties of undefined (reading 'pathname')` + ); + }); +}); + +describe('link prop.to - [prod mode]', () => { + beforeAll(async () => { + ctx = await serveFixture('basic', { ssr: true }); + }); + afterAll(async () => { + await ctx.close(); + }); + afterEach(async () => { + await page.close(); + }); + + test(`downgrade fatal crashes`, async () => { + page = await ctx.browser.page(ctx.url('/fatal-link-demo')); + + // 1. without causing an immediate page crash. + expect(await page.$text('#button-link-without-to')).toContain( + 'Click to trigger a fatal error at runtime' + ); + + // 2. Only after user clicks , page re-render and display the "Internal Application Error" page. + await page.click('#button-link-without-to'); + expect(await page.$text('#__APP')).toContain('Internal Application Error'); + }); +}); diff --git a/test/fixtures/basic/src/routes/fatal-link-demo/page.js b/test/fixtures/basic/src/routes/fatal-link-demo/page.js new file mode 100644 index 000000000..8441a513c --- /dev/null +++ b/test/fixtures/basic/src/routes/fatal-link-demo/page.js @@ -0,0 +1,17 @@ +import * as React from 'react'; +import { Link } from '@shuvi/runtime'; + +// @note test purpose to trigger runtime error +const TO = undefined; + +export default function Page() { + return ( +
+ Demo runtime error - Link component missing required `prop.to` +
+ +
+ ); +}