fix(routing): don't trigger get of headers #12937
Conversation
🦋 Changeset detectedLatest commit: f6483fe The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
github-actions
bot
added
the
pkg: astro
| | ReadableStream | ||
| | URLSearchParams | ||
| | FormData | ||
| | ReadableStream<Uint8Array>; |
There was a problem hiding this comment.
This is type provided by Request.body
There was a problem hiding this comment.
Could we use Request['body'] instead?
|
|
||
| export interface CreateRequestOptions { | ||
| url: URL | string; | ||
| clientAddress?: string | undefined; | ||
| headers: HeaderType; | ||
| method?: string; | ||
| body?: RequestBody | undefined; | ||
| body?: RequestBody | undefined | null; |
There was a problem hiding this comment.
I added null because Request.body is ReadableStream<Uint8Array> | null
CodSpeed Performance ReportMerging #12937 will not alter performanceComparing Summary
|
packages/astro/src/core/request.ts
Outdated
| * @param newUrl The new `URL` | ||
| * @param oldRequest The old `Request` | ||
| */ | ||
| export function copyRequest(newUrl: URL, oldRequest: Request, isPrerendered: boolean): Request { |
There was a problem hiding this comment.
Where is this version of the function used?
There was a problem hiding this comment.
Bad copy-paste. Good catch
| 'astro': patch | ||
| --- | ||
|
|
||
| Fixes an issue where the use of `Astro.rewrite` would trigger the invalid use of `Astro.request.headers` |
There was a problem hiding this comment.
Are these two separate issues beign fixed? It's a little unclear.
There was a problem hiding this comment.
Yes. What should I reword?
There was a problem hiding this comment.
That's fine. Just wanted to confirm that they're separate.
Changes
Closes #12931
Closes PLT-2736
The issue was that doing
oldRequest.headersduring a rewrite was causing the warning to fire, because it triggers the.getmethod.This is fixed now by checking the
isPrerenderedflag.Also, I updated the method
copyRequestto use the internal methodcopyRequest, which creates theheadersobject based onisPrenredererd. This should fix cases (not caught, probably) where users could have usedRequest.headersduring a rewrite.Testing
I tested it locally using the reproduction provided.
Docs
N/A