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

Implement Rewrite Headers support during routing #3897

Merged
merged 5 commits into from
Feb 22, 2023
Merged

Conversation

jridgewell
Copy link
Contributor

@jridgewell jridgewell commented Feb 21, 2023

The final piece to supporting Rewrites is implementing support for overwriting response headers. These overwritten headers need to be collected until we resolve into a final ContentSourceContent::Static (which gets turned into ResolveSourceRequestResult, then GetFromSourceResult) to be returned into the HTTP response.

I chose to implement this as an external list from the headers already present in a ContentSourceContent::Static. It felt wrong to content.await? just to overwrite the returned headers, and then repackage that as a new ContentSourceContentVc. These header_overwrites exist only as a byproduct of route resolution, and specifically the exact path of rewrites we go through to get there. They're not intrinsic to the ContentSourceContent itself.

Fixes WEB-617
Fixes WEB-299

@vercel
Copy link

vercel bot commented Feb 21, 2023

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

Name Status Preview Comments Updated
examples-basic-web 🔄 Building (Inspect) Feb 21, 2023 at 11:05PM (UTC)
turbo-site ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 21, 2023 at 11:05PM (UTC)
8 Ignored Deployments
Name Status Preview Comments Updated
examples-cra-web ⬜️ Ignored (Inspect) Feb 21, 2023 at 11:05PM (UTC)
examples-designsystem-docs ⬜️ Ignored (Inspect) Feb 21, 2023 at 11:05PM (UTC)
examples-kitchensink-blog ⬜️ Ignored (Inspect) Feb 21, 2023 at 11:05PM (UTC)
examples-native-web ⬜️ Ignored (Inspect) Feb 21, 2023 at 11:05PM (UTC)
examples-nonmonorepo ⬜️ Ignored (Inspect) Feb 21, 2023 at 11:05PM (UTC)
examples-svelte-web ⬜️ Ignored (Inspect) Feb 21, 2023 at 11:05PM (UTC)
examples-tailwind-web ⬜️ Ignored (Inspect) Feb 21, 2023 at 11:05PM (UTC)
examples-vite-web ⬜️ Ignored (Inspect) Feb 21, 2023 at 11:05PM (UTC)

@github-actions
Copy link
Contributor

github-actions bot commented Feb 21, 2023

⚠️ CI failed ⚠️

The following steps have failed in CI:

  • Turbopack Rust tests (mac/win, non-blocking)

See workflow summary for details

Copy link
Member

@sokra sokra left a comment

Choose a reason for hiding this comment

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

Test case

@jridgewell
Copy link
Contributor Author

I'm working on the test cases for Rewrite/Redirect/Headers now.

@jridgewell
Copy link
Contributor Author

Tests implemented for Rewrite/Redirect, but the Headers support just landed in Next side and I can't test it until we update our canary dep.

@jridgewell
Copy link
Contributor Author

I've manually confirmed this works once by duplicating vercel/next.js@185a892.

@jridgewell jridgewell added the pr: automerge Kodiak will merge these automatically after checks pass label Feb 21, 2023
@github-actions
Copy link
Contributor

Benchmark for e0f37ec

Test Base PR % Significant %
bench_hydration/Turbopack SSR/1000 modules 3473.81ms ± 15.61ms 3423.26ms ± 9.02ms -1.46% -0.04%
Click to view full benchmark
Test Base PR % Significant %
bench_hmr_to_commit/Turbopack CSR/1000 modules 9537.06µs ± 58.86µs 9529.63µs ± 60.30µs -0.08%
bench_hmr_to_commit/Turbopack RCC/1000 modules 12.55ms ± 0.41ms 12.98ms ± 0.31ms +3.44%
bench_hmr_to_commit/Turbopack RSC/1000 modules 498.72ms ± 2.27ms 502.07ms ± 3.90ms +0.67%
bench_hmr_to_commit/Turbopack SSR/1000 modules 10.02ms ± 0.16ms 9737.87µs ± 81.14µs -2.83%
bench_hmr_to_eval/Turbopack CSR/1000 modules 8604.70µs ± 73.50µs 8638.32µs ± 62.19µs +0.39%
bench_hmr_to_eval/Turbopack RCC/1000 modules 11.07ms ± 0.33ms 10.99ms ± 0.40ms -0.68%
bench_hmr_to_eval/Turbopack SSR/1000 modules 8742.65µs ± 47.91µs 8790.32µs ± 65.19µs +0.55%
bench_hydration/Turbopack RCC/1000 modules 3692.96ms ± 12.60ms 3707.34ms ± 16.24ms +0.39%
bench_hydration/Turbopack RSC/1000 modules 3384.26ms ± 9.94ms 3369.09ms ± 9.43ms -0.45%
bench_hydration/Turbopack SSR/1000 modules 3473.81ms ± 15.61ms 3423.26ms ± 9.02ms -1.46% -0.04%
bench_startup/Turbopack CSR/1000 modules 2698.27ms ± 8.71ms 2673.68ms ± 10.63ms -0.91%
bench_startup/Turbopack RCC/1000 modules 2292.20ms ± 3.87ms 2308.17ms ± 6.29ms +0.70%
bench_startup/Turbopack RSC/1000 modules 2226.40ms ± 3.80ms 2223.27ms ± 7.82ms -0.14%
bench_startup/Turbopack SSR/1000 modules 2092.30ms ± 4.23ms 2092.50ms ± 1.86ms +0.01%

@jridgewell jridgewell merged commit c1ac2ee into main Feb 22, 2023
@jridgewell jridgewell deleted the jrl-response-header branch February 22, 2023 00:40
@jridgewell
Copy link
Contributor Author

Merging over some test flake.

@sokra
Copy link
Member

sokra commented Feb 22, 2023

Merging over some test flake.

kodiak could handle this now. It won't block on mac or windows CI anymore.

ijjk pushed a commit to vercel/next.js that referenced this pull request Feb 22, 2023
jridgewell added a commit to vercel/next.js that referenced this pull request Mar 10, 2023
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

4 participants