Skip to content

Commit

Permalink
feat: downgrade fatal crash error for <Link> component (#596)
Browse files Browse the repository at this point in the history
  • Loading branch information
evenchange4 authored Sep 12, 2024
1 parent 2cbfd70 commit 1205fc5
Show file tree
Hide file tree
Showing 5 changed files with 134 additions and 3 deletions.
2 changes: 1 addition & 1 deletion examples/basic/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"scripts": {
"dev": "shuvi dev",
"build": "shuvi build && npm run lint",
"start": "shuvi start",
"start": "shuvi serve",
"lint": "shuvi lint"
},
"dependencies": {
Expand Down
17 changes: 17 additions & 0 deletions examples/basic/src/routes/fatal-link-demo/page.tsx
Original file line number Diff line number Diff line change
@@ -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 (
<div>
Demo runtime error - Link component missing required `prop.to`
<br />
<button id="button-link-without-to">
<Link to={TO}>Click to trigger a fatal error at runtime</Link>
</button>
</div>
);
}
48 changes: 46 additions & 2 deletions packages/router-react/src/Link.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ function isModifiedEvent(event: React.MouseEvent) {
* <Link to="/about" a='a' b='b'>About</Link> => <{...rest} a>
* ```
*/
export const Link = React.forwardRef<HTMLAnchorElement, LinkProps>(
function LinkWithRef(
const BaseLink = React.forwardRef<HTMLAnchorElement, LinkProps>(
function BaseLinkWithRef(
{ onClick, replace: replaceProp = false, state, target, to, ...rest },
ref
) {
Expand Down Expand Up @@ -75,6 +75,50 @@ export const Link = React.forwardRef<HTMLAnchorElement, LinkProps>(
}
);

/**
* @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 <Link>, page re-render
* and display the "Internal Application Error" page.
*
* @issue https://github.com/shuvijs/shuvi/pull/596
*/
export const Link = React.forwardRef<HTMLAnchorElement, LinkProps>(
function LinkWithRef(props, ref) {
const invalidPropTo = typeof props.to === 'undefined';
if (invalidPropTo) {
console.error(
`The prop 'to' is required in '<Link>', but its value is 'undefined'`,
JSON.stringify({ props })
);
}

const [downgradeError, setDowngradeError] = React.useState(
process.env.NODE_ENV === 'production'
);

if (downgradeError && invalidPropTo) {
return (
<a
{...props}
onClick={e => {
e.preventDefault();
setDowngradeError(false);
}}
ref={ref}
/>
);
}

return <BaseLink {...props} ref={ref} />;
}
);

export interface LinkProps
extends Omit<React.AnchorHTMLAttributes<HTMLAnchorElement>, 'href'> {
replace?: boolean;
Expand Down
53 changes: 53 additions & 0 deletions test/e2e/link-without-to-props.test.ts
Original file line number Diff line number Diff line change
@@ -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 <Link>, 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');
});
});
17 changes: 17 additions & 0 deletions test/fixtures/basic/src/routes/fatal-link-demo/page.js
Original file line number Diff line number Diff line change
@@ -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 (
<div>
Demo runtime error - Link component missing required `prop.to`
<br />
<button id="button-link-without-to">
<Link to={TO}>Click to trigger a fatal error at runtime</Link>
</button>
</div>
);
}

0 comments on commit 1205fc5

Please sign in to comment.