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.

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

Fix server generation of 404 pages #3103

Merged
merged 1 commit into from Dec 21, 2022
Merged

Fix server generation of 404 pages #3103

merged 1 commit into from Dec 21, 2022

Conversation

jridgewell
Copy link
Contributor

@jridgewell jridgewell commented Dec 21, 2022

Any 404.js page would inject a [...] path into the server generation. The problem comes from the pathname_for_path function, which will extract the extension from that path. In this case, it leads to a basename of [... and an extension of ], breaking the regular_expression_for_path dynamic route parser.

Unfortunately, our integration tests timeout when I try to write a basic test case for this, so it's not possible to test at the moment. I've manually verified.

Fixes WEB-214

@jridgewell jridgewell requested a review from a team as a code owner December 21, 2022 02:09
@vercel
Copy link

vercel bot commented Dec 21, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
turbo-site ✅ Ready (Inspect) Visit Preview Dec 21, 2022 at 2:09AM (UTC)
7 Ignored Deployments
Name Status Preview Updated
examples-basic-web ⬜️ Ignored (Inspect) Dec 21, 2022 at 2:09AM (UTC)
examples-cra-web ⬜️ Ignored (Inspect) Dec 21, 2022 at 2:09AM (UTC)
examples-designsystem-docs ⬜️ Ignored (Inspect) Dec 21, 2022 at 2:09AM (UTC)
examples-kitchensink-blog ⬜️ Ignored (Inspect) Dec 21, 2022 at 2:09AM (UTC)
examples-native-web ⬜️ Ignored (Inspect) Dec 21, 2022 at 2:09AM (UTC)
examples-nonmonorepo ⬜️ Ignored (Inspect) Dec 21, 2022 at 2:09AM (UTC)
examples-svelte-web ⬜️ Ignored (Inspect) Dec 21, 2022 at 2:09AM (UTC)

@github-actions
Copy link
Contributor

🟢 CI successful 🟢

Thanks

@github-actions
Copy link
Contributor

Benchmark for abb3a73

Click to view benchmark
Test Base PR % Significant %
bench_hmr_to_commit/Turbopack CSR/1000 modules 7992.51µs ± 63.89µs 8036.22µs ± 54.62µs +0.55%
bench_hmr_to_commit/Turbopack RCC/1000 modules 8223.69µs ± 71.41µs 8263.68µs ± 57.64µs +0.49%
bench_hmr_to_commit/Turbopack RSC/1000 modules 757.97ms ± 2.93ms 760.32ms ± 2.67ms +0.31%
bench_hmr_to_commit/Turbopack SSR/1000 modules 8026.23µs ± 54.38µs 8067.53µs ± 86.96µs +0.51%
bench_hmr_to_eval/Turbopack CSR/1000 modules 6984.01µs ± 63.57µs 7002.48µs ± 47.05µs +0.26%
bench_hmr_to_eval/Turbopack RCC/1000 modules 7153.73µs ± 50.99µs 7264.83µs ± 58.91µs +1.55%
bench_hmr_to_eval/Turbopack SSR/1000 modules 7091.54µs ± 69.89µs 7058.80µs ± 56.65µs -0.46%
bench_hydration/Turbopack RCC/1000 modules 3264.63ms ± 13.42ms 3263.51ms ± 10.20ms -0.03%
bench_hydration/Turbopack RSC/1000 modules 2721.99ms ± 6.41ms 2708.40ms ± 8.40ms -0.50%
bench_hydration/Turbopack SSR/1000 modules 2562.06ms ± 8.42ms 2590.17ms ± 7.99ms +1.10%
bench_startup/Turbopack CSR/1000 modules 1588.68ms ± 3.22ms 1586.96ms ± 5.60ms -0.11%
bench_startup/Turbopack RCC/1000 modules 2456.28ms ± 6.06ms 2458.38ms ± 8.07ms +0.09%
bench_startup/Turbopack RSC/1000 modules 2335.65ms ± 7.09ms 2342.68ms ± 5.80ms +0.30%
bench_startup/Turbopack SSR/1000 modules 2016.09ms ± 4.14ms 2007.46ms ± 2.47ms -0.43%

@jridgewell jridgewell added the pr: automerge Kodiak will merge these automatically after checks pass label Dec 21, 2022
@kodiakhq kodiakhq bot merged commit 770a227 into main Dec 21, 2022
@kodiakhq kodiakhq bot deleted the jrl-404 branch December 21, 2022 07:38
jridgewell added a commit to vercel/next.js that referenced this pull request Mar 10, 2023
Any `404.js` page would inject a `[...]` path into the server generation. The problem comes from the `pathname_for_path` function, which will extract the extension from that path. In this case, it leads to a basename of `[...` and an extension of `]`, breaking the `regular_expression_for_path` dynamic route parser.

Unfortunately, our integration tests timeout when I try to write a basic test case for this, so it's not possible to test at the moment. I've manually verified.

Fixes WEB-214
sokra pushed a commit to vercel/next.js that referenced this pull request Mar 13, 2023
Any `404.js` page would inject a `[...]` path into the server generation. The problem comes from the `pathname_for_path` function, which will extract the extension from that path. In this case, it leads to a basename of `[...` and an extension of `]`, breaking the `regular_expression_for_path` dynamic route parser.

Unfortunately, our integration tests timeout when I try to write a basic test case for this, so it's not possible to test at the moment. I've manually verified.

Fixes WEB-214
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: automerge Kodiak will merge these automatically after checks pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants