Skip to content

Commit

Permalink
chore(middleware): lift restriction around serializable data (#7174)
Browse files Browse the repository at this point in the history
  • Loading branch information
ematipico committed May 23, 2023
1 parent d73c908 commit 92d1f01
Show file tree
Hide file tree
Showing 5 changed files with 7 additions and 113 deletions.
5 changes: 5 additions & 0 deletions .changeset/fuzzy-cycles-trade.md
@@ -0,0 +1,5 @@
---
'astro': patch
---

Remove restriction around serialisable data for `Astro.locals`
8 changes: 0 additions & 8 deletions packages/astro/src/core/endpoint/index.ts
Expand Up @@ -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');

Expand Down Expand Up @@ -117,12 +115,6 @@ export async function callEndpoint<MiddlewareResult = Response | EndpointOutput>
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);
}
);
Expand Down
26 changes: 0 additions & 26 deletions packages/astro/src/core/errors/errors-data.ts
Expand Up @@ -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
/**
Expand Down
66 changes: 2 additions & 64 deletions packages/astro/src/core/render/core.ts
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
}
15 changes: 0 additions & 15 deletions packages/astro/test/middleware.test.js
Expand Up @@ -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);
Expand Down Expand Up @@ -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');
Expand Down

0 comments on commit 92d1f01

Please sign in to comment.