Skip to content
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

chore(middleware): lift restriction around serializable data #7174

Merged
merged 2 commits into from May 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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