summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGravatar Emanuele Stoppa <my.burning@gmail.com> 2024-12-06 10:31:23 +0000
committerGravatar GitHub <noreply@github.com> 2024-12-06 10:31:23 +0000
commit8f8f15ca1d51a112b7344787ac02180e0aeb04bb (patch)
treeda0ca86c415cc6cf2e29fe92896c0e2b1b347f92
parenta71e9b93b317edc0ded49d4d50f1b7841c8cd428 (diff)
downloadastro-8f8f15ca1d51a112b7344787ac02180e0aeb04bb.tar.gz
astro-8f8f15ca1d51a112b7344787ac02180e0aeb04bb.tar.zst
astro-8f8f15ca1d51a112b7344787ac02180e0aeb04bb.zip
fix(routing): don't attach locals to request (#12647)
* fix(routing): don't attach locals to request * apply feedback
-rw-r--r--packages/astro/src/core/app/index.ts1
-rw-r--r--packages/astro/src/core/base-pipeline.ts3
-rw-r--r--packages/astro/src/core/middleware/index.ts16
-rw-r--r--packages/astro/src/core/request.ts5
-rw-r--r--packages/astro/src/vite-plugin-astro-server/route.ts3
-rw-r--r--packages/astro/test/units/routing/api-context.test.js17
-rw-r--r--packages/astro/test/units/vite-plugin-astro-server/response.test.js12
7 files changed, 37 insertions, 20 deletions
diff --git a/packages/astro/src/core/app/index.ts b/packages/astro/src/core/app/index.ts
index 26a6523d8..36543b5ca 100644
--- a/packages/astro/src/core/app/index.ts
+++ b/packages/astro/src/core/app/index.ts
@@ -259,7 +259,6 @@ export class App {
this.#logger.error(null, error.stack!);
return this.#renderError(request, { status: 500, error, clientAddress });
}
- Reflect.set(request, clientLocalsSymbol, locals);
}
if (!routeData) {
routeData = this.match(request);
diff --git a/packages/astro/src/core/base-pipeline.ts b/packages/astro/src/core/base-pipeline.ts
index 545cf5ed2..81cc553ba 100644
--- a/packages/astro/src/core/base-pipeline.ts
+++ b/packages/astro/src/core/base-pipeline.ts
@@ -1,4 +1,3 @@
-import { setGetEnv } from '../env/runtime.js';
import { createI18nMiddleware } from '../i18n/middleware.js';
import type { ComponentInstance } from '../types/astro.js';
import type { MiddlewareHandler, RewritePayload } from '../types/public/common.js';
@@ -10,8 +9,6 @@ import type {
SSRResult,
} from '../types/public/internal.js';
import { createOriginCheckMiddleware } from './app/middlewares.js';
-import { AstroError } from './errors/errors.js';
-import { AstroErrorData } from './errors/index.js';
import type { Logger } from './logger/core.js';
import { NOOP_MIDDLEWARE_FN } from './middleware/noop-middleware.js';
import { sequence } from './middleware/sequence.js';
diff --git a/packages/astro/src/core/middleware/index.ts b/packages/astro/src/core/middleware/index.ts
index c7ed6e647..7f4c2867e 100644
--- a/packages/astro/src/core/middleware/index.ts
+++ b/packages/astro/src/core/middleware/index.ts
@@ -39,6 +39,11 @@ export type CreateContext = {
* User defined default locale
*/
defaultLocale: string;
+
+ /**
+ * Initial value of the locals
+ */
+ locals: App.Locals;
};
/**
@@ -49,6 +54,7 @@ function createContext({
params = {},
userDefinedLocales = [],
defaultLocale = '',
+ locals,
}: CreateContext): APIContext {
let preferredLocale: string | undefined = undefined;
let preferredLocaleList: string[] | undefined = undefined;
@@ -104,15 +110,15 @@ function createContext({
return clientIpAddress;
},
get locals() {
- let locals = Reflect.get(request, clientLocalsSymbol);
+ // TODO: deprecate this usage. This is used only by the edge middleware for now, so its usage should be basically none.
+ let _locals = locals ?? Reflect.get(request, clientLocalsSymbol);
if (locals === undefined) {
- locals = {};
- Reflect.set(request, clientLocalsSymbol, locals);
+ _locals = {};
}
- if (typeof locals !== 'object') {
+ if (typeof _locals !== 'object') {
throw new AstroError(AstroErrorData.LocalsNotAnObject);
}
- return locals;
+ return _locals;
},
set locals(_) {
throw new AstroError(AstroErrorData.LocalsReassigned);
diff --git a/packages/astro/src/core/request.ts b/packages/astro/src/core/request.ts
index b8cc89fb8..302370ba8 100644
--- a/packages/astro/src/core/request.ts
+++ b/packages/astro/src/core/request.ts
@@ -24,8 +24,6 @@ export interface CreateRequestOptions {
routePattern: string;
}
-const clientLocalsSymbol = Symbol.for('astro.locals');
-
/**
* Used by astro internals to create a web standard request object.
*
@@ -39,7 +37,6 @@ export function createRequest({
method = 'GET',
body = undefined,
logger,
- locals,
isPrerendered = false,
routePattern,
}: CreateRequestOptions): Request {
@@ -93,7 +90,5 @@ export function createRequest({
});
}
- Reflect.set(request, clientLocalsSymbol, locals ?? {});
-
return request;
}
diff --git a/packages/astro/src/vite-plugin-astro-server/route.ts b/packages/astro/src/vite-plugin-astro-server/route.ts
index a3a5bc93c..b4b659805 100644
--- a/packages/astro/src/vite-plugin-astro-server/route.ts
+++ b/packages/astro/src/vite-plugin-astro-server/route.ts
@@ -160,6 +160,7 @@ export async function handleRoute({
let mod: ComponentInstance | undefined = undefined;
let route: RouteData;
const middleware = (await loadMiddleware(loader)).onRequest;
+ // This is required for adapters to set locals in dev mode. They use a dev server middleware to inject locals to the `http.IncomingRequest` object.
const locals = Reflect.get(incomingRequest, clientLocalsSymbol);
const { preloadedComponent } = matchedRoute;
@@ -242,7 +243,7 @@ export async function handleRoute({
const fourOhFourRoute = await matchRoute('/404', manifestData, pipeline);
if (fourOhFourRoute) {
renderContext = await RenderContext.create({
- locals,
+ locals: {},
pipeline,
pathname,
middleware: isDefaultPrerendered404(fourOhFourRoute.route) ? undefined : middleware,
diff --git a/packages/astro/test/units/routing/api-context.test.js b/packages/astro/test/units/routing/api-context.test.js
index 1e955e50c..027a70f8b 100644
--- a/packages/astro/test/units/routing/api-context.test.js
+++ b/packages/astro/test/units/routing/api-context.test.js
@@ -16,4 +16,21 @@ describe('createAPIContext', () => {
assert.equal(context.clientAddress, '192.0.2.43');
});
+
+ it('should return the correct locals', () => {
+ const request = new Request('http://example.com', {
+ headers: {
+ 'x-forwarded-for': '192.0.2.43, 172.16.58.3',
+ },
+ });
+
+ const context = createContext({
+ request,
+ locals: {
+ foo: 'bar',
+ },
+ });
+
+ assert.deepEqual(context.locals, { foo: 'bar' });
+ });
});
diff --git a/packages/astro/test/units/vite-plugin-astro-server/response.test.js b/packages/astro/test/units/vite-plugin-astro-server/response.test.js
index 2d256fb71..60f583939 100644
--- a/packages/astro/test/units/vite-plugin-astro-server/response.test.js
+++ b/packages/astro/test/units/vite-plugin-astro-server/response.test.js
@@ -87,16 +87,18 @@ describe('endpoints', () => {
url: '/streaming',
});
- const locals = { cancelledByTheServer: false };
- req[Symbol.for('astro.locals')] = locals;
-
container.handle(req, res);
await new Promise((resolve) => setTimeout(resolve, 500));
res.emit('close');
- await done;
+ try {
+ await done;
- assert.equal(locals.cancelledByTheServer, true);
+ assert.ok(true);
+
+ } catch (err) {
+ assert.fail(err)
+ }
});
});