-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Deprecate simple objects from endpoints #8132
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
--- | ||
'astro': patch | ||
--- | ||
|
||
Deprecate returning simple objects from endpoints. Endpoints should only return a `Response`. | ||
|
||
To return a result with a custom encoding not supported by a `Response`, you can use the `ResponseWithEncoding` utility class instead. | ||
|
||
Before: | ||
|
||
```ts | ||
export function GET() { | ||
return { | ||
body: '...', | ||
encoding: 'binary', | ||
}; | ||
} | ||
``` | ||
|
||
After: | ||
|
||
```ts | ||
export function GET({ ResponseWithEncoding }) { | ||
return new ResponseWithEncoding('...', undefined, 'binary'); | ||
} | ||
``` |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,7 @@ import type { | |
MiddlewareHandler, | ||
Params, | ||
} from '../../@types/astro'; | ||
import mime from 'mime'; | ||
import type { Environment, RenderContext } from '../render/index'; | ||
import { renderEndpoint } from '../../runtime/server/index.js'; | ||
import { ASTRO_VERSION } from '../constants.js'; | ||
|
@@ -14,19 +15,11 @@ import { AstroError, AstroErrorData } from '../errors/index.js'; | |
import { warn } from '../logger/core.js'; | ||
import { callMiddleware } from '../middleware/callMiddleware.js'; | ||
|
||
const encoder = new TextEncoder(); | ||
|
||
const clientAddressSymbol = Symbol.for('astro.clientAddress'); | ||
const clientLocalsSymbol = Symbol.for('astro.locals'); | ||
|
||
export type EndpointCallResult = | ||
| (EndpointOutput & { | ||
type: 'simple'; | ||
cookies: AstroCookies; | ||
}) | ||
| { | ||
type: 'response'; | ||
response: Response; | ||
}; | ||
|
||
type CreateAPIContext = { | ||
request: Request; | ||
params: Params; | ||
|
@@ -62,6 +55,7 @@ export function createAPIContext({ | |
}, | ||
}); | ||
}, | ||
ResponseWithEncoding, | ||
url: new URL(request.url), | ||
get clientAddress() { | ||
if (!(clientAddressSymbol in request)) { | ||
|
@@ -96,12 +90,37 @@ export function createAPIContext({ | |
return context; | ||
} | ||
|
||
type ResponseParameters = ConstructorParameters<typeof Response>; | ||
|
||
export class ResponseWithEncoding extends Response { | ||
constructor(body: ResponseParameters[0], init: ResponseParameters[1], encoding?: BufferEncoding) { | ||
// If a body string is given, try to encode it to preserve the behaviour as simple objects. | ||
// We don't do the full handling as simple objects so users can control how headers are set instead. | ||
if (typeof body === 'string') { | ||
// In NodeJS, we can use Buffer.from which supports all BufferEncoding | ||
if (typeof Buffer !== 'undefined' && Buffer.from) { | ||
body = Buffer.from(body, encoding); | ||
} | ||
// In non-NodeJS, use the web-standard TextEncoder for utf-8 strings | ||
else if (encoding == null || encoding === 'utf8' || encoding === 'utf-8') { | ||
body = encoder.encode(body); | ||
} | ||
} | ||
|
||
super(body, init); | ||
|
||
if (encoding) { | ||
this.headers.set('X-Astro-Encoding', encoding); | ||
} | ||
} | ||
} | ||
|
||
export async function callEndpoint<MiddlewareResult = Response | EndpointOutput>( | ||
mod: EndpointHandler, | ||
env: Environment, | ||
ctx: RenderContext, | ||
onRequest?: MiddlewareHandler<MiddlewareResult> | undefined | ||
): Promise<EndpointCallResult> { | ||
): Promise<Response> { | ||
const context = createAPIContext({ | ||
request: ctx.request, | ||
params: ctx.params, | ||
|
@@ -124,15 +143,30 @@ export async function callEndpoint<MiddlewareResult = Response | EndpointOutput> | |
response = await renderEndpoint(mod, context, env.ssr, env.logging); | ||
} | ||
|
||
const isEndpointSSR = env.ssr && !ctx.route?.prerender; | ||
|
||
if (response instanceof Response) { | ||
if (isEndpointSSR && response.headers.get('X-Astro-Encoding')) { | ||
warn( | ||
env.logging, | ||
'ssr', | ||
'`ResponseWithEncoding` is ignored in SSR. Please return an instance of Response. See https://docs.astro.build/en/core-concepts/endpoints/#server-endpoints-api-routes for more information.' | ||
); | ||
} | ||
attachCookiesToResponse(response, context.cookies); | ||
return { | ||
type: 'response', | ||
response, | ||
}; | ||
return response; | ||
} | ||
|
||
if (env.ssr && !ctx.route?.prerender) { | ||
// The endpoint returned a simple object, convert it to a Response | ||
|
||
// TODO: Remove in Astro 4.0 | ||
warn( | ||
env.logging, | ||
'astro', | ||
`${ctx.route.component} returns a simple object which is deprecated. Please return an instance of Response. See https://docs.astro.build/en/core-concepts/endpoints/#server-endpoints-api-routes for more information.` | ||
); | ||
|
||
if (isEndpointSSR) { | ||
if (response.hasOwnProperty('headers')) { | ||
warn( | ||
env.logging, | ||
|
@@ -150,9 +184,58 @@ export async function callEndpoint<MiddlewareResult = Response | EndpointOutput> | |
} | ||
} | ||
|
||
return { | ||
...response, | ||
type: 'simple', | ||
cookies: context.cookies, | ||
}; | ||
let body: BodyInit; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. All the pipeline simple object handling converges into this function starting from this line. This should also make the dev/prod behaviour more consistent. |
||
const headers = new Headers(); | ||
|
||
// Try to get the MIME type for this route | ||
const pathname = ctx.route | ||
? // Try the static route `pathname` | ||
ctx.route.pathname ?? | ||
// Dynamic routes don't include `pathname`, so synthesize a path for these (e.g. 'src/pages/[slug].svg') | ||
ctx.route.segments.map((s) => s.map((p) => p.content).join('')).join('/') | ||
: // Fallback to pathname of the request | ||
ctx.pathname; | ||
const mimeType = mime.getType(pathname) || 'text/plain'; | ||
headers.set('Content-Type', `${mimeType};charset=utf-8`); | ||
|
||
// Save encoding to X-Astro-Encoding to be used later during SSG with `fs.writeFile`. | ||
// It won't work in SSR and is already warned above. | ||
if (response.encoding) { | ||
headers.set('X-Astro-Encoding', response.encoding); | ||
} | ||
|
||
// For Uint8Array (binary), it can passed to Response directly | ||
if (response.body instanceof Uint8Array) { | ||
body = response.body; | ||
headers.set('Content-Length', body.byteLength.toString()); | ||
} | ||
// In NodeJS, we can use Buffer.from which supports all BufferEncoding | ||
else if (typeof Buffer !== 'undefined' && Buffer.from) { | ||
body = Buffer.from(response.body, response.encoding); | ||
headers.set('Content-Length', body.byteLength.toString()); | ||
} | ||
// In non-NodeJS, use the web-standard TextEncoder for utf-8 strings only | ||
// to calculate the content length | ||
else if ( | ||
response.encoding == null || | ||
response.encoding === 'utf8' || | ||
response.encoding === 'utf-8' | ||
) { | ||
body = encoder.encode(response.body); | ||
headers.set('Content-Length', body.byteLength.toString()); | ||
} | ||
// Fallback pass it to Response directly. It will mainly rely on X-Astro-Encoding | ||
// to be further processed in SSG. | ||
else { | ||
body = response.body; | ||
// NOTE: Can't calculate the content length as we can't encode to figure out the real length. | ||
// But also because we don't need the length for SSG as it's only being written to disk. | ||
} | ||
|
||
response = new Response(body, { | ||
status: 200, | ||
headers, | ||
}); | ||
attachCookiesToResponse(response, context.cookies); | ||
return response; | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this was a mistake in #8003, we technically only support binary strings, and not binary unit8arrays. We have a test for binary strings and the PR caused a type error.
But simple objects are being deprecated anyways so I don't think it's a big deal.