From 92d1f017e5c0a921973e028b90c7975e74dce433 Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Tue, 23 May 2023 15:10:30 +0100 Subject: [PATCH] chore(middleware): lift restriction around serializable data (#7174) --- .changeset/fuzzy-cycles-trade.md | 5 ++ packages/astro/src/core/endpoint/index.ts | 8 --- packages/astro/src/core/errors/errors-data.ts | 26 -------- packages/astro/src/core/render/core.ts | 66 +------------------ packages/astro/test/middleware.test.js | 15 ----- 5 files changed, 7 insertions(+), 113 deletions(-) create mode 100644 .changeset/fuzzy-cycles-trade.md diff --git a/.changeset/fuzzy-cycles-trade.md b/.changeset/fuzzy-cycles-trade.md new file mode 100644 index 000000000000..cfea1a58f306 --- /dev/null +++ b/.changeset/fuzzy-cycles-trade.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Remove restriction around serialisable data for `Astro.locals` diff --git a/packages/astro/src/core/endpoint/index.ts b/packages/astro/src/core/endpoint/index.ts index 7d298a80226d..890ed3a733f9 100644 --- a/packages/astro/src/core/endpoint/index.ts +++ b/packages/astro/src/core/endpoint/index.ts @@ -16,8 +16,6 @@ import { AstroCookies, attachToResponse } from '../cookies/index.js'; import { AstroError, AstroErrorData } from '../errors/index.js'; import { warn, type LogOptions } from '../logger/core.js'; import { callMiddleware } from '../middleware/callMiddleware.js'; -import { isValueSerializable } from '../render/core.js'; - const clientAddressSymbol = Symbol.for('astro.clientAddress'); const clientLocalsSymbol = Symbol.for('astro.locals'); @@ -117,12 +115,6 @@ export async function callEndpoint onRequest, context, async () => { - if (env.mode === 'development' && !isValueSerializable(context.locals)) { - throw new AstroError({ - ...AstroErrorData.LocalsNotSerializable, - message: AstroErrorData.LocalsNotSerializable.message(ctx.pathname), - }); - } return await renderEndpoint(mod, context, env.ssr); } ); diff --git a/packages/astro/src/core/errors/errors-data.ts b/packages/astro/src/core/errors/errors-data.ts index 15d0f7056e33..a97c18d0e819 100644 --- a/packages/astro/src/core/errors/errors-data.ts +++ b/packages/astro/src/core/errors/errors-data.ts @@ -693,32 +693,6 @@ See https://docs.astro.build/en/guides/server-side-rendering/ for more informati '`locals` can only be assigned to an object. Other values like numbers, strings, etc. are not accepted.', hint: 'If you tried to remove some information from the `locals` object, try to use `delete` or set the property to `undefined`.', }, - - /** - * @docs - * @description - * Thrown in development mode when a user attempts to store something that is not serializable in `locals`. - * - * For example: - * ```ts - * import {defineMiddleware} from "astro/middleware"; - * export const onRequest = defineMiddleware((context, next) => { - * context.locals = { - * foo() { - * alert("Hello world!") - * } - * }; - * return next(); - * }); - * ``` - */ - LocalsNotSerializable: { - title: '`Astro.locals` is not serializable', - code: 3034, - message: (href: string) => { - return `The information stored in \`Astro.locals\` for the path "${href}" is not serializable.\nMake sure you store only serializable data.`; - }, - }, // No headings here, that way Vite errors are merged with Astro ones in the docs, which makes more sense to users. // Vite Errors - 4xxx /** diff --git a/packages/astro/src/core/render/core.ts b/packages/astro/src/core/render/core.ts index 2d6fb088ee27..24bec9d30e29 100644 --- a/packages/astro/src/core/render/core.ts +++ b/packages/astro/src/core/render/core.ts @@ -116,16 +116,8 @@ export async function renderPage({ mod, renderContext, env, apiContext }: Render if (!Component) throw new Error(`Expected an exported Astro component but received typeof ${typeof Component}`); - let locals = {}; - if (apiContext) { - if (env.mode === 'development' && !isValueSerializable(apiContext.locals)) { - throw new AstroError({ - ...AstroErrorData.LocalsNotSerializable, - message: AstroErrorData.LocalsNotSerializable.message(renderContext.pathname), - }); - } - locals = apiContext.locals; - } + let locals = apiContext?.locals ?? {}; + const result = createResult({ adapterName: env.adapterName, links: renderContext.links, @@ -171,57 +163,3 @@ export async function renderPage({ mod, renderContext, env, apiContext }: Render return response; } - -/** - * Checks whether any value can is serializable. - * - * A serializable value contains plain values. For example, `Proxy`, `Set`, `Map`, functions, etc. - * are not serializable objects. - * - * @param object - */ -export function isValueSerializable(value: unknown): boolean { - let type = typeof value; - let plainObject = true; - if (type === 'object' && isPlainObject(value)) { - for (const [, nestedValue] of Object.entries(value)) { - if (!isValueSerializable(nestedValue)) { - plainObject = false; - break; - } - } - } else { - plainObject = false; - } - let result = - value === null || - type === 'string' || - type === 'number' || - type === 'boolean' || - Array.isArray(value) || - plainObject; - - return result; -} - -/** - * - * From [redux-toolkit](https://github.com/reduxjs/redux-toolkit/blob/master/packages/toolkit/src/isPlainObject.ts) - * - * Returns true if the passed value is "plain" object, i.e. an object whose - * prototype is the root `Object.prototype`. This includes objects created - * using object literals, but not for instance for class instances. - */ -function isPlainObject(value: unknown): value is object { - if (typeof value !== 'object' || value === null) return false; - - let proto = Object.getPrototypeOf(value); - if (proto === null) return true; - - let baseProto = proto; - while (Object.getPrototypeOf(baseProto) !== null) { - baseProto = Object.getPrototypeOf(baseProto); - } - - return proto === baseProto; -} diff --git a/packages/astro/test/middleware.test.js b/packages/astro/test/middleware.test.js index 8e2e0dd0af80..270666678ec1 100644 --- a/packages/astro/test/middleware.test.js +++ b/packages/astro/test/middleware.test.js @@ -61,12 +61,6 @@ describe('Middleware in DEV mode', () => { expect($('p').html()).to.equal('Not interested'); }); - it('should throw an error when locals are not serializable', async () => { - let html = await fixture.fetch('/broken-locals').then((res) => res.text()); - let $ = cheerio.load(html); - expect($('title').html()).to.equal('LocalsNotSerializable'); - }); - it("should throw an error when the middleware doesn't call next or doesn't return a response", async () => { let html = await fixture.fetch('/does-nothing').then((res) => res.text()); let $ = cheerio.load(html); @@ -180,15 +174,6 @@ describe('Middleware API in PROD mode, SSR', () => { expect($('p').html()).to.equal('Not interested'); }); - it('should NOT throw an error when locals are not serializable', async () => { - const app = await fixture.loadTestAdapterApp(); - const request = new Request('http://example.com/broken-locals'); - const response = await app.render(request); - const html = await response.text(); - const $ = cheerio.load(html); - expect($('title').html()).to.not.equal('LocalsNotSerializable'); - }); - it("should throws an error when the middleware doesn't call next or doesn't return a response", async () => { const app = await fixture.loadTestAdapterApp(); const request = new Request('http://example.com/does-nothing');