Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

error in error handling #896

Merged
merged 3 commits into from
Sep 24, 2024
+222 −2
Merged

error in error handling #896

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8e41304
test: add fixture for testing broken links
pmelab Sep 19, 2024
e6e3f6e
fix: add html and body tags to global error boundary
pmelab Sep 19, 2024
a83f4c9
Merge branch 'main' into error-in-error-handling
dai-shi Sep 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 86 additions & 0 deletions e2e/broken-link.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
import { debugChildProcess, getFreePort, terminate, test } from './utils.js';
import { fileURLToPath } from 'node:url';
import { cp, mkdtemp } from 'node:fs/promises';
import { exec, execSync } from 'node:child_process';
import { expect } from '@playwright/test';
import waitPort from 'wait-port';
import { join } from 'node:path';
import { tmpdir } from 'node:os';
import { createRequire } from 'node:module';

let standaloneDir: string;
const exampleDir = fileURLToPath(
new URL('./fixtures/broken-links', import.meta.url),
);
const wakuDir = fileURLToPath(new URL('../packages/waku', import.meta.url));
const { version } = createRequire(import.meta.url)(
join(wakuDir, 'package.json'),
);

async function start() {
const port = await getFreePort();
const cp = exec(
`node ${join(standaloneDir, './node_modules/waku/dist/cli.js')} start --port ${port}`,
{ cwd: standaloneDir },
);
debugChildProcess(cp, fileURLToPath(import.meta.url), [
/ExperimentalWarning: Custom ESM Loaders is an experimental feature and might change at any time/,
]);

await waitPort({ port });
return [port, cp.pid];
}

test.describe('broken links', async () => {
test.beforeEach(async () => {
// GitHub Action on Windows doesn't support mkdtemp on global temp dir,
// Which will cause files in `src` folder to be empty.
// I don't know why
const tmpDir = process.env.TEMP_DIR ? process.env.TEMP_DIR : tmpdir();
standaloneDir = await mkdtemp(join(tmpDir, 'waku-broken-link-'));
await cp(exampleDir, standaloneDir, {
filter: (src) => {
return !src.includes('node_modules') && !src.includes('dist');
},
recursive: true,
});
execSync(`pnpm pack --pack-destination ${standaloneDir}`, {
cwd: wakuDir,
stdio: 'inherit',
});
const name = `waku-${version}.tgz`;
execSync(`npm install ${join(standaloneDir, name)}`, {
cwd: standaloneDir,
stdio: 'inherit',
});
execSync(
`node ${join(standaloneDir, './node_modules/waku/dist/cli.js')} build`,
{
cwd: standaloneDir,
stdio: 'inherit',
},
);
});

test('correct link', async ({ page }) => {
const [port, pid] = await start();

await page.goto(`http://localhost:${port}`);

await page.getByRole('link', { name: 'Existing page' }).click();
await expect(page.getByRole('heading')).toHaveText('Existing page');

await terminate(pid!);
});

test('broken link', async ({ page }) => {
const [port, pid] = await start();

await page.goto(`http://localhost:${port}`);

await page.getByRole('link', { name: 'Broken link' }).click();
await expect(page.getByRole('heading')).toHaveText('Not Found');

await terminate(pid!);
});
});
22 changes: 22 additions & 0 deletions e2e/fixtures/broken-links/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
{
"name": "broken-links",
"version": "0.1.0",
"type": "module",
"private": true,
"scripts": {
"dev": "waku dev",
"build": "waku build",
"start": "waku start"
},
"dependencies": {
"react": "19.0.0-rc-d6cb4e77-20240911",
"react-dom": "19.0.0-rc-d6cb4e77-20240911",
"react-server-dom-webpack": "19.0.0-rc-d6cb4e77-20240911",
"waku": "workspace:*"
},
"devDependencies": {
"@types/react": "^18.3.5",
"@types/react-dom": "^18.3.0",
"typescript": "^5.6.2"
}
}
20 changes: 20 additions & 0 deletions e2e/fixtures/broken-links/src/pages/_layout.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import type { ReactNode } from 'react';

export default async function RootLayout({
children,
}: {
children: ReactNode;
}) {
return (
<html>
<head></head>
<body>{children}</body>
</html>
);
}

export const getConfig = async () => {
return {
render: 'static',
};
};
18 changes: 18 additions & 0 deletions e2e/fixtures/broken-links/src/pages/exists.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import { Link } from 'waku';

const Page = () => (
<div>
<h1>Existing page</h1>
<p>
<Link to="/">Back</Link>
</p>
</div>
);

export const getConfig = async () => {
return {
render: 'dynamic',
};
};

export default Page;
21 changes: 21 additions & 0 deletions e2e/fixtures/broken-links/src/pages/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { Link } from 'waku';

const Page = () => (
<div>
<h1>Index</h1>
<p>
<Link to="/exists">Existing page</Link>
</p>
<p>
<Link to="/broken">Broken link</Link>
</p>
</div>
);

export const getConfig = async () => {
return {
render: 'dynamic',
};
};

export default Page;
17 changes: 17 additions & 0 deletions e2e/fixtures/broken-links/tsconfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{
"compilerOptions": {
"composite": true,
"strict": true,
"target": "esnext",
"downlevelIteration": true,
"esModuleInterop": true,
"module": "nodenext",
"skipLibCheck": true,
"noUncheckedIndexedAccess": true,
"exactOptionalPropertyTypes": true,
"types": ["react/experimental"],
"jsx": "react-jsx",
"rootDir": "./src",
"outDir": "./dist"
}
}
12 changes: 10 additions & 2 deletions packages/waku/src/router/client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -551,6 +551,14 @@ export function ServerRouter({
);
}

function renderError(message: string) {
return createElement(
'html',
null,
createElement('body', null, createElement('h1', null, message)),
);
}
Comment on lines +554 to +560
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't feel like a final solution, but it looks good to me for now at least.


class ErrorBoundary extends Component<
{ children: ReactNode },
{ error?: unknown }
Expand All @@ -568,9 +576,9 @@ class ErrorBoundary extends Component<
this.state.error instanceof Error &&
(this.state.error as any).statusCode === 404
) {
return createElement('h1', null, 'Not Found');
return renderError('Not Found');
}
return createElement('h1', null, String(this.state.error));
return renderError(String(this.state.error));
}
return this.props.children;
}
Expand Down
25 changes: 25 additions & 0 deletions pnpm-lock.yaml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions tsconfig.e2e.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@
},
{
"path": "./e2e/fixtures/use-router/tsconfig.json"
},
{
"path": "./e2e/fixtures/broken-links/tsconfig.json"
}
]
}
Loading