Skip to content

Commit

Permalink
Merge branch 'feat/route-manifest-refactor' of https://github.com/wit…
Browse files Browse the repository at this point in the history
…hastro/astro into feat/route-manifest-refactor
  • Loading branch information
florian-lefebvre committed Dec 6, 2024
2 parents a4680c3 + 4885577 commit bdfbeae
Show file tree
Hide file tree
Showing 12 changed files with 74 additions and 27 deletions.
5 changes: 5 additions & 0 deletions .changeset/bright-gifts-prove.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"astro": patch
---

Updates a reference in an error message
5 changes: 5 additions & 0 deletions .changeset/unlucky-wasps-refuse.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@astrojs/markdown-remark': patch
---

Fixes frontmatter parsing if file is encoded in UTF8 with BOM
1 change: 0 additions & 1 deletion packages/astro/src/core/app/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,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);
Expand Down
3 changes: 0 additions & 3 deletions packages/astro/src/core/base-pipeline.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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';
Expand Down
2 changes: 1 addition & 1 deletion packages/astro/src/core/errors/errors-data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -955,7 +955,7 @@ export const AstroGlobNoMatch = {
/**
* @docs
* @see
* - [Astro.redirect](https://docs.astro.build/en/reference/api-reference/#astroredirect)
* - [Astro.redirect](https://docs.astro.build/en/reference/api-reference/#redirect)
* @description
* A redirect must be given a location with the `Location` header.
*/
Expand Down
16 changes: 11 additions & 5 deletions packages/astro/src/core/middleware/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@ export type CreateContext = {
* User defined default locale
*/
defaultLocale: string;

/**
* Initial value of the locals
*/
locals: App.Locals;
};

/**
Expand All @@ -49,6 +54,7 @@ function createContext({
params = {},
userDefinedLocales = [],
defaultLocale = '',
locals,
}: CreateContext): APIContext {
let preferredLocale: string | undefined = undefined;
let preferredLocaleList: string[] | undefined = undefined;
Expand Down Expand Up @@ -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);
Expand Down
5 changes: 0 additions & 5 deletions packages/astro/src/core/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand All @@ -39,7 +37,6 @@ export function createRequest({
method = 'GET',
body = undefined,
logger,
locals,
isPrerendered = false,
routePattern,
}: CreateRequestOptions): Request {
Expand Down Expand Up @@ -93,7 +90,5 @@ export function createRequest({
});
}

Reflect.set(request, clientLocalsSymbol, locals ?? {});

return request;
}
3 changes: 2 additions & 1 deletion packages/astro/src/vite-plugin-astro-server/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down
17 changes: 17 additions & 0 deletions packages/astro/test/units/routing/api-context.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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' });
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -87,16 +87,17 @@ 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);
}
});
});
7 changes: 4 additions & 3 deletions packages/markdown/remark/src/frontmatter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,10 @@ export function isFrontmatterValid(frontmatter: Record<string, any>) {
}

// Capture frontmatter wrapped with `---`, including any characters and new lines within it.
// Only capture if it exists near the top of the file (whitespaces between the start of file and
// the start of `---` are allowed)
const frontmatterRE = /^\s*---([\s\S]*?\n)---/;
// Only capture if `---` exists near the top of the file, including:
// 1. Start of file (including if has BOM encoding)
// 2. Start of file with any whitespace (but `---` must still start on a new line)
const frontmatterRE = /(?:^\uFEFF?|^\s*\n)---([\s\S]*?\n)---/;
export function extractFrontmatter(code: string): string | undefined {
return frontmatterRE.exec(code)?.[1];
}
Expand Down
26 changes: 23 additions & 3 deletions packages/markdown/remark/test/frontmatter.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,21 @@ import assert from 'node:assert/strict';
import { describe, it } from 'node:test';
import { extractFrontmatter, parseFrontmatter } from '../dist/index.js';

const bom = '\uFEFF';

describe('extractFrontmatter', () => {
it('works', () => {
const yaml = `\nfoo: bar\n`;
assert.equal(extractFrontmatter(`---${yaml}---`), yaml);
assert.equal(extractFrontmatter(` ---${yaml}---`), yaml);
assert.equal(extractFrontmatter(`${bom}---${yaml}---`), yaml);
assert.equal(extractFrontmatter(`\n---${yaml}---`), yaml);
assert.equal(extractFrontmatter(`\n \n---${yaml}---`), yaml);
assert.equal(extractFrontmatter(`---${yaml}---\ncontent`), yaml);
assert.equal(extractFrontmatter(`${bom}---${yaml}---\ncontent`), yaml);
assert.equal(extractFrontmatter(`\n\n---${yaml}---\n\ncontent`), yaml);
assert.equal(extractFrontmatter(`\n \n---${yaml}---\n\ncontent`), yaml);
assert.equal(extractFrontmatter(` ---${yaml}---`), undefined);
assert.equal(extractFrontmatter(`---${yaml} ---`), undefined);
assert.equal(extractFrontmatter(`text\n---${yaml}---\n\ncontent`), undefined);
});
});
Expand All @@ -24,10 +29,10 @@ describe('parseFrontmatter', () => {
rawFrontmatter: yaml,
content: '',
});
assert.deepEqual(parseFrontmatter(` ---${yaml}---`), {
assert.deepEqual(parseFrontmatter(`${bom}---${yaml}---`), {
frontmatter: { foo: 'bar' },
rawFrontmatter: yaml,
content: ' ',
content: bom,
});
assert.deepEqual(parseFrontmatter(`\n---${yaml}---`), {
frontmatter: { foo: 'bar' },
Expand All @@ -44,6 +49,11 @@ describe('parseFrontmatter', () => {
rawFrontmatter: yaml,
content: '\ncontent',
});
assert.deepEqual(parseFrontmatter(`${bom}---${yaml}---\ncontent`), {
frontmatter: { foo: 'bar' },
rawFrontmatter: yaml,
content: `${bom}\ncontent`,
});
assert.deepEqual(parseFrontmatter(`\n\n---${yaml}---\n\ncontent`), {
frontmatter: { foo: 'bar' },
rawFrontmatter: yaml,
Expand All @@ -54,6 +64,16 @@ describe('parseFrontmatter', () => {
rawFrontmatter: yaml,
content: '\n \n\n\ncontent',
});
assert.deepEqual(parseFrontmatter(` ---${yaml}---`), {
frontmatter: {},
rawFrontmatter: '',
content: ` ---${yaml}---`,
});
assert.deepEqual(parseFrontmatter(`---${yaml} ---`), {
frontmatter: {},
rawFrontmatter: '',
content: `---${yaml} ---`,
});
assert.deepEqual(parseFrontmatter(`text\n---${yaml}---\n\ncontent`), {
frontmatter: {},
rawFrontmatter: '',
Expand Down

0 comments on commit bdfbeae

Please sign in to comment.