Skip to content

Commit

Permalink
Switch app.render signature (#9199)
Browse files Browse the repository at this point in the history
* feat(app): app.render optional object

* tests

* update vercel and node

* update changeset

* deprecation notice and loggin

* clarify changeset

* add node, vercel changeset

* deduplicate code
  • Loading branch information
lilnasy authored Nov 29, 2023
1 parent c421a3d commit 49aa215
Show file tree
Hide file tree
Showing 8 changed files with 80 additions and 10 deletions.
6 changes: 6 additions & 0 deletions .changeset/big-cooks-notice.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@astrojs/vercel': major
'@astrojs/node': major
---

The internals of the integration have been updated to support Astro 4.0. Make sure to upgrade your Astro version as Astro 3.0 is no longer supported.
20 changes: 20 additions & 0 deletions .changeset/sharp-starfishes-compete.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
---
'astro': major
---

This change only affects maintainers of third-party adapters. In the Integration API, the `app.render()` method of the `App` class has been simplified.

Instead of two optional arguments, it now takes a single optional argument that is an object with two optional properties: `routeData` and `locals`.
```diff
app.render(request)

- app.render(request, routeData)
+ app.render(request, { routeData })

- app.render(request, routeData, locals)
+ app.render(request, { routeData, locals })

- app.render(request, undefined, locals)
+ app.render(request, { locals })
```
The current signature is deprecated but will continue to function until next major version.
40 changes: 38 additions & 2 deletions packages/astro/src/core/app/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,11 @@ const responseSentSymbol = Symbol.for('astro.responseSent');

const STATUS_CODES = new Set([404, 500]);

export interface RenderOptions {
routeData?: RouteData;
locals?: object;
}

export interface RenderErrorOptions {
routeData?: RouteData;
response?: Response;
Expand All @@ -59,6 +64,7 @@ export class App {
#baseWithoutTrailingSlash: string;
#pipeline: SSRRoutePipeline;
#adapterLogger: AstroIntegrationLogger;
#renderOptionsDeprecationWarningShown = false;

constructor(manifest: SSRManifest, streaming = true) {
this.#manifest = manifest;
Expand Down Expand Up @@ -141,7 +147,32 @@ export class App {
return routeData;
}

async render(request: Request, routeData?: RouteData, locals?: object): Promise<Response> {
async render(request: Request, options?: RenderOptions): Promise<Response>
/**
* @deprecated Instead of passing `RouteData` and locals individually, pass an object with `routeData` and `locals` properties.
* See https://github.com/withastro/astro/pull/9199 for more information.
*/
async render(request: Request, routeData?: RouteData, locals?: object): Promise<Response>
async render(request: Request, routeDataOrOptions?: RouteData | RenderOptions, maybeLocals?: object): Promise<Response> {
let routeData: RouteData | undefined;
let locals: object | undefined;

if (routeDataOrOptions && ('routeData' in routeDataOrOptions || 'locals' in routeDataOrOptions)) {
if ('routeData' in routeDataOrOptions) {
routeData = routeDataOrOptions.routeData;
}
if ('locals' in routeDataOrOptions) {
locals = routeDataOrOptions.locals;
}
}
else {
routeData = routeDataOrOptions as RouteData | undefined;
locals = maybeLocals;
if (routeDataOrOptions || locals) {
this.#logRenderOptionsDeprecationWarning();
}
}

// Handle requests with duplicate slashes gracefully by cloning with a cleaned-up request URL
if (request.url !== collapseDuplicateSlashes(request.url)) {
request = new Request(collapseDuplicateSlashes(request.url), request);
Expand All @@ -152,7 +183,6 @@ export class App {
if (!routeData) {
return this.#renderError(request, { status: 404 });
}

Reflect.set(request, clientLocalsSymbol, locals ?? {});
const pathname = this.#getPathnameFromRequest(request);
const defaultStatus = this.#getDefaultStatusCode(routeData, pathname);
Expand Down Expand Up @@ -210,6 +240,12 @@ export class App {
return response;
}

#logRenderOptionsDeprecationWarning() {
if (this.#renderOptionsDeprecationWarningShown) return;
this.#logger.warn("deprecated", `The adapter ${this.#manifest.adapterName} is using a deprecated signature of the 'app.render()' method. From Astro 4.0, locals and routeData are provided as properties on an optional object to this method. Using the old signature will cause an error in Astro 5.0. See https://github.com/withastro/astro/pull/9199 for more information.`)
this.#renderOptionsDeprecationWarningShown = true;
}

setCookieHeaders(response: Response) {
return getSetCookiesFromResponse(response);
}
Expand Down
12 changes: 10 additions & 2 deletions packages/astro/src/core/app/node.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type { RouteData } from '../../@types/astro.js';
import type { SerializedSSRManifest, SSRManifest } from './types.js';
import type { RenderOptions } from './index.js';

import * as fs from 'node:fs';
import { IncomingMessage } from 'node:http';
Expand Down Expand Up @@ -116,11 +117,18 @@ export class NodeApp extends App {
}
return super.match(req);
}
render(req: NodeIncomingMessage | Request, routeData?: RouteData, locals?: object) {
render(request: NodeIncomingMessage | Request, options?: RenderOptions): Promise<Response>
/**
* @deprecated Instead of passing `RouteData` and locals individually, pass an object with `routeData` and `locals` properties.
* See https://github.com/withastro/astro/pull/9199 for more information.
*/
render(request: NodeIncomingMessage | Request, routeData?: RouteData, locals?: object): Promise<Response>
render(req: NodeIncomingMessage | Request, routeDataOrOptions?: RouteData | RenderOptions, maybeLocals?: object) {
if (!(req instanceof Request)) {
req = createRequestFromNodeRequest(req);
}
return super.render(req, routeData, locals);
// @ts-expect-error The call would have succeeded against the implementation, but implementation signatures of overloads are not externally visible.
return super.render(req, routeDataOrOptions, maybeLocals);
}
}

Expand Down
2 changes: 1 addition & 1 deletion packages/astro/test/middleware.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ describe('Middleware API in PROD mode, SSR', () => {
it('should correctly call the middleware function for 404', async () => {
const request = new Request('http://example.com/funky-url');
const routeData = app.match(request);
const response = await app.render(request, routeData);
const response = await app.render(request, { routeData });
const text = await response.text();
expect(text.includes('Error')).to.be.true;
expect(text.includes('bar')).to.be.true;
Expand Down
2 changes: 1 addition & 1 deletion packages/astro/test/ssr-locals.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ describe('SSR Astro.locals from server', () => {
const app = await fixture.loadTestAdapterApp();
const request = new Request('http://example.com/foo');
const locals = { foo: 'bar' };
const response = await app.render(request, undefined, locals);
const response = await app.render(request, { locals });
const html = await response.text();

const $ = cheerio.load(html);
Expand Down
6 changes: 3 additions & 3 deletions packages/integrations/node/src/nodeMiddleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,10 @@ export default function (app: NodeApp, mode: Options['mode']) {
}

try {
const route = app.match(req);
if (route) {
const routeData = app.match(req);
if (routeData) {
try {
const response = await app.render(req, route, locals);
const response = await app.render(req, { routeData, locals });
await writeWebResponse(app, res, response);
} catch (err: unknown) {
if (next) {
Expand Down
2 changes: 1 addition & 1 deletion packages/integrations/vercel/src/serverless/entrypoint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export const createExports = (manifest: SSRManifest) => {
locals = JSON.parse(localsAsString);
}
}
await setResponse(app, res, await app.render(request, routeData, locals));
await setResponse(app, res, await app.render(request, { routeData, locals }));
};

return { default: handler };
Expand Down

0 comments on commit 49aa215

Please sign in to comment.