From 49aa215a01ee1c4805316c85bb0aea6cfbc25a31 Mon Sep 17 00:00:00 2001 From: Arsh <69170106+lilnasy@users.noreply.github.com> Date: Wed, 29 Nov 2023 15:38:42 +0000 Subject: [PATCH] Switch `app.render` signature (#9199) * 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 --- .changeset/big-cooks-notice.md | 6 +++ .changeset/sharp-starfishes-compete.md | 20 ++++++++++ packages/astro/src/core/app/index.ts | 40 ++++++++++++++++++- packages/astro/src/core/app/node.ts | 12 +++++- packages/astro/test/middleware.test.js | 2 +- packages/astro/test/ssr-locals.test.js | 2 +- .../integrations/node/src/nodeMiddleware.ts | 6 +-- .../vercel/src/serverless/entrypoint.ts | 2 +- 8 files changed, 80 insertions(+), 10 deletions(-) create mode 100644 .changeset/big-cooks-notice.md create mode 100644 .changeset/sharp-starfishes-compete.md diff --git a/.changeset/big-cooks-notice.md b/.changeset/big-cooks-notice.md new file mode 100644 index 0000000000..9a0586c73a --- /dev/null +++ b/.changeset/big-cooks-notice.md @@ -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. diff --git a/.changeset/sharp-starfishes-compete.md b/.changeset/sharp-starfishes-compete.md new file mode 100644 index 0000000000..0eb4f413bc --- /dev/null +++ b/.changeset/sharp-starfishes-compete.md @@ -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. diff --git a/packages/astro/src/core/app/index.ts b/packages/astro/src/core/app/index.ts index aa89c82868..ee50fc3435 100644 --- a/packages/astro/src/core/app/index.ts +++ b/packages/astro/src/core/app/index.ts @@ -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; @@ -59,6 +64,7 @@ export class App { #baseWithoutTrailingSlash: string; #pipeline: SSRRoutePipeline; #adapterLogger: AstroIntegrationLogger; + #renderOptionsDeprecationWarningShown = false; constructor(manifest: SSRManifest, streaming = true) { this.#manifest = manifest; @@ -141,7 +147,32 @@ export class App { return routeData; } - async render(request: Request, routeData?: RouteData, locals?: object): Promise { + async render(request: Request, options?: RenderOptions): Promise + /** + * @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 + async render(request: Request, routeDataOrOptions?: RouteData | RenderOptions, maybeLocals?: object): Promise { + 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); @@ -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); @@ -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); } diff --git a/packages/astro/src/core/app/node.ts b/packages/astro/src/core/app/node.ts index a127b9831c..1c4ced2b40 100644 --- a/packages/astro/src/core/app/node.ts +++ b/packages/astro/src/core/app/node.ts @@ -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'; @@ -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 + /** + * @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 + 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); } } diff --git a/packages/astro/test/middleware.test.js b/packages/astro/test/middleware.test.js index a6745f1765..c858f37a82 100644 --- a/packages/astro/test/middleware.test.js +++ b/packages/astro/test/middleware.test.js @@ -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; diff --git a/packages/astro/test/ssr-locals.test.js b/packages/astro/test/ssr-locals.test.js index 41e5710fbb..68e6289908 100644 --- a/packages/astro/test/ssr-locals.test.js +++ b/packages/astro/test/ssr-locals.test.js @@ -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); diff --git a/packages/integrations/node/src/nodeMiddleware.ts b/packages/integrations/node/src/nodeMiddleware.ts index 4fd0a4bc23..0b9381f1d0 100644 --- a/packages/integrations/node/src/nodeMiddleware.ts +++ b/packages/integrations/node/src/nodeMiddleware.ts @@ -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) { diff --git a/packages/integrations/vercel/src/serverless/entrypoint.ts b/packages/integrations/vercel/src/serverless/entrypoint.ts index 7b548dc37b..513c34640a 100644 --- a/packages/integrations/vercel/src/serverless/entrypoint.ts +++ b/packages/integrations/vercel/src/serverless/entrypoint.ts @@ -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 };