withastro / roadmap

Ideas, suggestions, and formal RFC proposals for the Astro project.
292 stars 29 forks source link

Astro.response, a way to modify response properties #183

Closed matthewp closed 2 years ago

matthewp commented 2 years ago

Summary

Adds an Astro.response object as a way to modify response properties, such as the status, statusText, and headers. The object mirrors the structure of the standard Response but is a plain object, to allow modifying properties that are readonly on Response.

Links

FredKSchott commented 2 years ago

I'm overall onboard with this, since the Response object is clearly not designed for this "modify the response as you go" use-case. My main concerns are around how we message this API. I really don't want to give anyone the impression of "oh look, Astro implemented their own Response object, that's not hashtag-web-standards-use-the-platform".

I noticed that the three options you define (status, statusText, headers) map 1:1 with the init argument of the response: https://developer.mozilla.org/en-US/docs/Web/API/Response. Is one option to say that that's what Astro.response is? That would mean that we'd type it as the standard JS init type, and never extend it past what JS supports. That would solve my concern around messaging ("look, we're still using web standards!").

A valid alternative would be to just expose Astro.headers and Astro.status similar to how you've pitched Astro.cookies. I say this as the person who's probably been pushing for hardest for "raw response access", but now that I see how limited it is, I see how headers and status are the only two use-cases that we can really support here.

(aside: do you think it is time to consider an Astro.context namespace for these helpers?) (also aside: do we need statusText in a world of HTML-rendered return bodies?)

matthewp commented 2 years ago

Ah yeah, I really like the Response init symmetry as well, I didn't realize this, so let me see if this is something there are already typings for and update the PR to reflect. This also means we can pass this object directly into calling new Response. This makes it feel nice and low-level.


I did think about Astro.headers, etc. but I thought that might be confused with the request headers. To be more clear it would probably need to be Astro.responseHeaders in which case, why not just make it be a sub-property on Astro.response.

matthewp commented 2 years ago

So TypeScript does come with this interface:

interface ResponseInit {
    headers?: HeadersInit;
    status?: number;
    statusText?: string;
}

I'll update the PR to reflect this.