0
Fork 0
mirror of https://github.com/withastro/astro.git synced 2025-01-27 22:19:04 -05:00

fix(Astro global): parity with ctx.site (#10325)

* move `createResult()` into `RenderContext`

* `Astro.site`-`ctx.site` parity

* shared `clientAdress()` implementation

* remove base from `manifest.site`

* astro global tests

* add api context tests

* add changeset

* flaky test

* error on `Astro.response.headers` reassignment
This commit is contained in:
Arsh 2024-03-07 20:14:53 +05:30 committed by GitHub
parent fdd5bf277e
commit f33cce8f6c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
16 changed files with 229 additions and 303 deletions

View file

@ -0,0 +1,5 @@
---
"astro": patch
---
Fixes an issue where `ctx.site` included the configured `base` in API routes and middleware, unlike `Astro.site` in astro pages.

View file

@ -2701,7 +2701,7 @@ export interface SSRResult {
slots: Record<string, any> | null
): AstroGlobal;
resolve: (s: string) => Promise<string>;
response: ResponseInit;
response: AstroGlobal["response"];
renderers: SSRLoadedRenderer[];
/**
* Map of directive name (e.g. `load`) to the directive script code

View file

@ -45,7 +45,7 @@ export abstract class Pipeline {
/**
* Used for `Astro.site`.
*/
readonly site = manifest.site
readonly site = manifest.site ? new URL(manifest.site) : undefined,
) {
this.internalMiddleware = [
createI18nMiddleware(i18n, manifest.base, manifest.trailingSlash, manifest.buildFormat),

View file

@ -609,9 +609,7 @@ function createBuildManifest(
renderers,
base: settings.config.base,
assetsPrefix: settings.config.build.assetsPrefix,
site: settings.config.site
? new URL(settings.config.base, settings.config.site).toString()
: settings.config.site,
site: settings.config.site,
componentMetadata: internals.componentMetadata,
i18n: i18nManifest,
buildFormat: settings.config.build.format,

View file

@ -56,6 +56,8 @@ export async function compile({
normalizedFilename: normalizeFilename(filename, astroConfig.root),
sourcemap: 'both',
internalURL: 'astro/compiler-runtime',
// TODO: this is no longer neccessary for `Astro.site`
// but it somehow allows working around caching issues in content collections for some tests
astroGlobalArgs: JSON.stringify(astroConfig.site),
scopedStyleStrategy: astroConfig.scopedStyleStrategy,
resultScopedSlot: true,

View file

@ -792,7 +792,7 @@ export const MiddlewareNotAResponse = {
* @docs
* @description
*
* Thrown in development mode when `locals` is overwritten with something that is not an object
* Thrown when `locals` is overwritten with something that is not an object
*
* For example:
* ```ts
@ -811,6 +811,19 @@ export const LocalsNotAnObject = {
hint: 'If you tried to remove some information from the `locals` object, try to use `delete` or set the property to `undefined`.',
} satisfies ErrorData;
/**
* @docs
* @description
* Thrown when a value is being set as the `headers` field on the `ResponseInit` object available as `Astro.response`.
*/
export const AstroResponseHeadersReassigned = {
name: 'AstroResponseHeadersReassigned',
title: '`Astro.response.headers` must not be reassigned.',
message:
'Individual headers can be added to and removed from `Astro.response.headers`, but it must not be replaced with another instance of `Headers` altogether.',
hint: 'Consider using `Astro.response.headers.add()`, and `Astro.response.headers.delete()`.',
} satisfies ErrorData;
/**
* @docs
* @description

View file

@ -74,8 +74,6 @@ function createContext({
return (currentLocale ??= computeCurrentLocale(
route,
userDefinedLocales,
undefined,
undefined
));
},
url,

View file

@ -1,8 +1,11 @@
import type {
APIContext,
AstroGlobal,
AstroGlobalPartial,
ComponentInstance,
MiddlewareHandler,
RouteData,
SSRResult,
} from '../@types/astro.js';
import {
computeCurrentLocale,
@ -11,21 +14,20 @@ import {
} from '../i18n/utils.js';
import { renderEndpoint } from '../runtime/server/endpoint.js';
import { renderPage } from '../runtime/server/index.js';
import { renderRedirect } from './redirects/render.js';
import {
ASTRO_VERSION,
REROUTE_DIRECTIVE_HEADER,
ROUTE_TYPE_HEADER,
clientAddressSymbol,
clientLocalsSymbol,
responseSentSymbol,
} from './constants.js';
import { attachCookiesToResponse } from './cookies/index.js';
import { AstroCookies } from './cookies/index.js';
import { attachCookiesToResponse, AstroCookies } from './cookies/index.js';
import { AstroError, AstroErrorData } from './errors/index.js';
import { callMiddleware } from './middleware/callMiddleware.js';
import { sequence } from './middleware/index.js';
import { renderRedirect } from './redirects/render.js';
import { createResult } from './render/index.js';
import { type Pipeline, getParams, getProps } from './render/index.js';
import { type Pipeline, getParams, getProps, Slots } from './render/index.js';
export class RenderContext {
private constructor(
@ -135,38 +137,15 @@ export class RenderContext {
const generator = `Astro v${ASTRO_VERSION}`;
const redirect = (path: string, status = 302) =>
new Response(null, { status, headers: { Location: path } });
const site = pipeline.site ? new URL(pipeline.site) : undefined;
return {
cookies,
get clientAddress() {
return renderContext.clientAddress()
},
get currentLocale() {
return renderContext.computeCurrentLocale();
},
generator,
params,
get preferredLocale() {
return renderContext.computePreferredLocale();
},
get preferredLocaleList() {
return renderContext.computePreferredLocaleList();
},
props,
redirect,
request,
site,
url,
get clientAddress() {
if (clientAddressSymbol in request) {
return Reflect.get(request, clientAddressSymbol) as string;
}
if (pipeline.adapterName) {
throw new AstroError({
...AstroErrorData.ClientAddressNotAvailable,
message: AstroErrorData.ClientAddressNotAvailable.message(pipeline.adapterName),
});
} else {
throw new AstroError(AstroErrorData.StaticClientAddressNotAvailable);
}
},
get locals() {
return renderContext.locals;
},
@ -181,53 +160,142 @@ export class RenderContext {
Reflect.set(request, clientLocalsSymbol, val);
}
},
params,
get preferredLocale() {
return renderContext.computePreferredLocale();
},
get preferredLocaleList() {
return renderContext.computePreferredLocaleList();
},
props,
redirect,
request,
site: pipeline.site,
url,
};
}
async createResult(mod: ComponentInstance) {
const { cookies, locals, params, pathname, pipeline, request, routeData, status } = this;
const { cookies, pathname, pipeline, routeData, status } = this;
const {
adapterName,
clientDirectives,
compressHTML,
i18n,
manifest,
logger,
renderers,
resolve,
site,
serverLike,
resolve
} = pipeline;
const { links, scripts, styles } = await pipeline.headElements(routeData);
const componentMetadata =
(await pipeline.componentMetadata(routeData)) ?? manifest.componentMetadata;
const { defaultLocale, locales, strategy } = i18n ?? {};
const headers = new Headers({ 'Content-Type': 'text/html' });
const partial = Boolean(mod.partial);
return createResult({
adapterName,
const response = {
status,
statusText: 'OK',
get headers() {
return headers
},
// Disallow `Astro.response.headers = new Headers`
set headers(_) {
throw new AstroError(AstroErrorData.AstroResponseHeadersReassigned);
}
} satisfies AstroGlobal["response"];
// Create the result object that will be passed into the renderPage function.
// This object starts here as an empty shell (not yet the result) but then
// calling the render() function will populate the object with scripts, styles, etc.
const result: SSRResult = {
clientDirectives,
componentMetadata,
compressHTML,
cookies,
defaultLocale,
locales,
locals,
logger,
/** This function returns the `Astro` faux-global */
createAstro: (astroGlobal, props, slots) => this.createAstro(result, astroGlobal, props, slots),
links,
params,
partial,
pathname,
renderers,
resolve,
request,
route: routeData.route,
strategy,
site,
response,
scripts,
ssr: serverLike,
status,
styles,
});
_metadata: {
hasHydrationScript: false,
rendererSpecificHydrationScripts: new Set(),
hasRenderedHead: false,
hasDirectives: new Set(),
headInTree: false,
extraHead: [],
propagators: new Set(),
},
};
return result;
}
createAstro(
result: SSRResult,
astroGlobalPartial: AstroGlobalPartial,
props: Record<string, any>,
slotValues: Record<string, any> | null
): AstroGlobal {
const renderContext = this;
const { cookies, locals, params, pipeline, request, url } = this;
const { response } = result;
const redirect = (path: string, status = 302) => {
// If the response is already sent, error as we cannot proceed with the redirect.
if ((request as any)[responseSentSymbol]) {
throw new AstroError({
...AstroErrorData.ResponseSentError,
});
}
return new Response(null, { status, headers: { Location: path } });
}
const slots = new Slots(result, slotValues, pipeline.logger) as unknown as AstroGlobal['slots'];
// `Astro.self` is added by the compiler
const astroGlobalCombined: Omit<AstroGlobal, 'self'> = {
...astroGlobalPartial,
cookies,
get clientAddress() {
return renderContext.clientAddress()
},
get currentLocale() {
return renderContext.computeCurrentLocale();
},
params,
get preferredLocale() {
return renderContext.computePreferredLocale();
},
get preferredLocaleList() {
return renderContext.computePreferredLocaleList();
},
props,
locals,
redirect,
request,
response,
slots,
site: pipeline.site,
url,
};
return astroGlobalCombined as AstroGlobal
}
clientAddress() {
const { pipeline, request } = this;
if (clientAddressSymbol in request) {
return Reflect.get(request, clientAddressSymbol) as string;
}
if (pipeline.adapterName) {
throw new AstroError({
...AstroErrorData.ClientAddressNotAvailable,
message: AstroErrorData.ClientAddressNotAvailable.message(pipeline.adapterName),
});
} else {
throw new AstroError(AstroErrorData.StaticClientAddressNotAvailable);
}
}
/**
@ -242,13 +310,21 @@ export class RenderContext {
routeData,
} = this;
if (!i18n) return;
const { defaultLocale, locales, strategy } = i18n;
return (this.#currentLocale ??= computeCurrentLocale(
routeData.route,
locales,
strategy,
defaultLocale
));
const fallbackTo = (
strategy === 'pathname-prefix-other-locales' ||
strategy === 'domains-prefix-other-locales'
) ? defaultLocale : undefined
// TODO: look into why computeCurrentLocale() needs routeData.route to pass ctx.currentLocale tests,
// and url.pathname to pass Astro.currentLocale tests.
// A single call with `routeData.pathname ?? routeData.route` as the pathname still fails.
return (this.#currentLocale ??=
computeCurrentLocale(routeData.route, locales) ??
computeCurrentLocale(url.pathname, locales) ??
fallbackTo);
}
#preferredLocale: APIContext['preferredLocale'];

View file

@ -3,7 +3,7 @@ import type { Pipeline } from '../base-pipeline.js';
export { Pipeline } from '../base-pipeline.js';
export { getParams, getProps } from './params-and-props.js';
export { loadRenderer } from './renderer.js';
export { createResult } from './result.js';
export { Slots } from './result.js';
export interface SSROptions {
/** The pipeline instance */

View file

@ -1,68 +1,17 @@
import type {
AstroGlobal,
AstroGlobalPartial,
Locales,
Params,
SSRElement,
SSRLoadedRenderer,
SSRResult,
} from '../../@types/astro.js';
import {
type RoutingStrategies,
computeCurrentLocale,
computePreferredLocale,
computePreferredLocaleList,
} from '../../i18n/utils.js';
import type { SSRResult } from '../../@types/astro.js';
import { type ComponentSlots, renderSlotToString } from '../../runtime/server/index.js';
import { renderJSX } from '../../runtime/server/jsx.js';
import { chunkToString } from '../../runtime/server/render/index.js';
import { clientAddressSymbol, responseSentSymbol } from '../constants.js';
import { AstroCookies } from '../cookies/index.js';
import { AstroError, AstroErrorData } from '../errors/index.js';
import type { Logger } from '../logger/core.js';
export interface CreateResultArgs {
/**
* Used to provide better error messages for `Astro.clientAddress`
*/
adapterName: string | undefined;
/**
* Value of Astro config's `output` option, true if "server" or "hybrid"
*/
ssr: boolean;
logger: Logger;
params: Params;
pathname: string;
renderers: SSRLoadedRenderer[];
clientDirectives: Map<string, string>;
compressHTML: boolean;
partial: boolean;
resolve: (s: string) => Promise<string>;
/**
* Used for `Astro.site`
*/
site: string | undefined;
links: Set<SSRElement>;
scripts: Set<SSRElement>;
styles: Set<SSRElement>;
componentMetadata: SSRResult['componentMetadata'];
request: Request;
status: number;
locals: App.Locals;
cookies: AstroCookies;
locales: Locales | undefined;
defaultLocale: string | undefined;
route: string;
strategy: RoutingStrategies | undefined;
}
function getFunctionExpression(slot: any) {
if (!slot) return;
if (slot.expressions?.length !== 1) return;
return slot.expressions[0] as (...args: any[]) => any;
}
class Slots {
export class Slots {
#result: SSRResult;
#slots: ComponentSlots | null;
#logger: Logger;
@ -131,157 +80,3 @@ class Slots {
return outHTML;
}
}
export function createResult(args: CreateResultArgs): SSRResult {
const { params, request, resolve, locals } = args;
const url = new URL(request.url);
const headers = new Headers();
headers.set('Content-Type', 'text/html');
const response: ResponseInit = {
status: args.status,
statusText: 'OK',
headers,
};
// Make headers be read-only
Object.defineProperty(response, 'headers', {
value: response.headers,
enumerable: true,
writable: false,
});
// Astro.cookies is defined lazily to avoid the cost on pages that do not use it.
let cookies: AstroCookies | undefined = args.cookies;
let preferredLocale: string | undefined = undefined;
let preferredLocaleList: string[] | undefined = undefined;
let currentLocale: string | undefined = undefined;
// Create the result object that will be passed into the render function.
// This object starts here as an empty shell (not yet the result) but then
// calling the render() function will populate the object with scripts, styles, etc.
const result: SSRResult = {
styles: args.styles ?? new Set<SSRElement>(),
scripts: args.scripts ?? new Set<SSRElement>(),
links: args.links ?? new Set<SSRElement>(),
componentMetadata: args.componentMetadata ?? new Map(),
renderers: args.renderers,
clientDirectives: args.clientDirectives,
compressHTML: args.compressHTML,
partial: args.partial,
pathname: args.pathname,
cookies,
/** This function returns the `Astro` faux-global */
createAstro(
astroGlobal: AstroGlobalPartial,
props: Record<string, any>,
slots: Record<string, any> | null
) {
const astroSlots = new Slots(result, slots, args.logger);
const Astro: AstroGlobal = {
// @ts-expect-error
__proto__: astroGlobal,
get clientAddress() {
if (!(clientAddressSymbol in request)) {
if (args.adapterName) {
throw new AstroError({
...AstroErrorData.ClientAddressNotAvailable,
message: AstroErrorData.ClientAddressNotAvailable.message(args.adapterName),
});
} else {
throw new AstroError(AstroErrorData.StaticClientAddressNotAvailable);
}
}
return Reflect.get(request, clientAddressSymbol) as string;
},
get cookies() {
if (cookies) {
return cookies;
}
cookies = new AstroCookies(request);
result.cookies = cookies;
return cookies;
},
get preferredLocale(): string | undefined {
if (preferredLocale) {
return preferredLocale;
}
if (args.locales) {
preferredLocale = computePreferredLocale(request, args.locales);
return preferredLocale;
}
return undefined;
},
get preferredLocaleList(): string[] | undefined {
if (preferredLocaleList) {
return preferredLocaleList;
}
if (args.locales) {
preferredLocaleList = computePreferredLocaleList(request, args.locales);
return preferredLocaleList;
}
return undefined;
},
get currentLocale(): string | undefined {
if (currentLocale) {
return currentLocale;
}
if (args.locales) {
currentLocale = computeCurrentLocale(
url.pathname,
args.locales,
args.strategy,
args.defaultLocale
);
if (currentLocale) {
return currentLocale;
}
}
return undefined;
},
params,
props,
locals,
request,
url,
redirect(path, status) {
// If the response is already sent, error as we cannot proceed with the redirect.
if ((request as any)[responseSentSymbol]) {
throw new AstroError({
...AstroErrorData.ResponseSentError,
});
}
return new Response(null, {
status: status || 302,
headers: {
Location: path,
},
});
},
response: response as AstroGlobal['response'],
slots: astroSlots as unknown as AstroGlobal['slots'],
};
return Astro;
},
resolve,
response,
_metadata: {
hasHydrationScript: false,
rendererSpecificHydrationScripts: new Set(),
hasRenderedHead: false,
hasDirectives: new Set(),
headInTree: false,
extraHead: [],
propagators: new Set(),
},
};
return result;
}

View file

@ -155,8 +155,6 @@ export function computePreferredLocaleList(request: Request, locales: Locales):
export function computeCurrentLocale(
pathname: string,
locales: Locales,
routingStrategy: RoutingStrategies | undefined,
defaultLocale: string | undefined
): undefined | string {
for (const segment of pathname.split('/')) {
for (const locale of locales) {
@ -179,15 +177,7 @@ export function computeCurrentLocale(
}
}
}
}
if (
routingStrategy === 'pathname-prefix-other-locales' ||
routingStrategy === 'domains-prefix-other-locales'
) {
return defaultLocale;
}
return undefined;
};
}
export type RoutingStrategies =

View file

@ -30,6 +30,8 @@ function createAstroGlobFn() {
// inside of getStaticPaths. See the `astroGlobalArgs` option for parameter type.
export function createAstro(site: string | undefined): AstroGlobalPartial {
return {
// TODO: this is no longer neccessary for `Astro.site`
// but it somehow allows working around caching issues in content collections for some tests
site: site ? new URL(site) : undefined,
generator: `Astro v${ASTRO_VERSION}`,
glob: createAstroGlobFn(),

View file

@ -136,9 +136,7 @@ export function createDevelopmentManifest(settings: AstroSettings): SSRManifest
renderers: [],
base: settings.config.base,
assetsPrefix: settings.config.build.assetsPrefix,
site: settings.config.site
? new URL(settings.config.base, settings.config.site).toString()
: settings.config.site,
site: settings.config.site,
componentMetadata: new Map(),
i18n: i18nManifest,
middleware(_, next) {

View file

@ -2,6 +2,7 @@ import assert from 'node:assert/strict';
import { after, before, describe, it } from 'node:test';
import * as cheerio from 'cheerio';
import { loadFixture } from './test-utils.js';
import testAdapter from './test-adapter.js';
describe('Astro Global', () => {
let fixture;
@ -9,7 +10,7 @@ describe('Astro Global', () => {
before(async () => {
fixture = await loadFixture({
root: './fixtures/astro-global/',
site: 'https://mysite.dev/blog/',
site: 'https://mysite.dev/subsite/',
base: '/blog',
});
});
@ -65,7 +66,7 @@ describe('Astro Global', () => {
it('Astro.site', async () => {
const html = await fixture.readFile('/index.html');
const $ = cheerio.load(html);
assert.equal($('#site').attr('href'), 'https://mysite.dev/blog/');
assert.equal($('#site').attr('href'), 'https://mysite.dev/subsite/');
});
it('Astro.glob() correctly returns an array of all posts', async () => {
@ -81,6 +82,30 @@ describe('Astro Global', () => {
assert.equal($('.post-url[href]').length, 8);
});
});
describe('app', () => {
/** @type {import('../dist/core/app/index.js').App} */
let app;
before(async () => {
fixture = await loadFixture({
root: './fixtures/astro-global/',
site: 'https://mysite.dev/subsite/',
base: '/new',
output: "server",
adapter: testAdapter()
});
await fixture.build();
app = await fixture.loadTestAdapterApp();
});
it('Astro.site', async () => {
const response = await app.render(new Request("https://example.com/"));
const html = await response.text();
const $ = cheerio.load(html);
assert.equal($('#site').attr('href'), 'https://mysite.dev/subsite/');
});
});
});
describe('Astro Global Defaults', () => {

View file

@ -5,20 +5,25 @@ import testAdapter from './test-adapter.js';
import { loadFixture } from './test-utils.js';
describe('API routes in SSR', () => {
/** @type {import('./test-utils').Fixture} */
let fixture;
const config = {
root: './fixtures/ssr-api-route/',
output: 'server',
site: 'https://mysite.dev/subsite/',
base: '/blog',
adapter: testAdapter(),
};
describe('Build', () => {
/** @type {import('./test-utils.js').App} */
let app;
before(async () => {
fixture = await loadFixture({
root: './fixtures/ssr-api-route/',
output: 'server',
adapter: testAdapter(),
});
const fixture = await loadFixture(config);
await fixture.build();
app = await fixture.loadTestAdapterApp();
});
it('Basic pages work', async () => {
const app = await fixture.loadTestAdapterApp();
const request = new Request('http://example.com/');
const response = await app.render(request);
const html = await response.text();
@ -26,7 +31,6 @@ describe('API routes in SSR', () => {
});
it('Can load the API route too', async () => {
const app = await fixture.loadTestAdapterApp();
const request = new Request('http://example.com/food.json');
const response = await app.render(request);
assert.equal(response.status, 200);
@ -35,7 +39,6 @@ describe('API routes in SSR', () => {
});
it('Has valid api context', async () => {
const app = await fixture.loadTestAdapterApp();
const request = new Request('http://example.com/context/any');
const response = await app.render(request);
assert.equal(response.status, 200);
@ -48,11 +51,17 @@ describe('API routes in SSR', () => {
assert.match(data.generator, /^Astro v/);
assert.equal(data.url, 'http://example.com/context/any');
assert.equal(data.clientAddress, '0.0.0.0');
assert.equal(data.site, "https://mysite.dev/subsite/");
});
})
describe('API Routes - Dev', () => {
describe('Dev', () => {
/** @type {import('./test-utils.js').DevServer} */
let devServer;
/** @type {import('./test-utils.js').Fixture} */
let fixture;
before(async () => {
fixture = await loadFixture(config);
devServer = await fixture.startDevServer();
});
@ -116,5 +125,20 @@ describe('API routes in SSR', () => {
assert.equal(count, 2, 'Found two seperate set-cookie response headers');
});
it('Has valid api context', async () => {
const response = await fixture.fetch('/context/any');
assert.equal(response.status, 200);
const data = await response.json();
assert.equal(data.cookiesExist, true);
assert.equal(data.requestExist, true);
assert.equal(data.redirectExist, true);
assert.equal(data.propsExist, true);
assert.deepEqual(data.params, { param: 'any' });
assert.match(data.generator, /^Astro v/);
assert.equal(data.url, 'http://[::1]:4321/blog/context/any');
assert.equal(data.clientAddress, '::1');
assert.equal(data.site, "https://mysite.dev/subsite/");
});
});
});

View file

@ -203,7 +203,7 @@ export function createBasicPipeline(options = {}) {
options.site
);
pipeline.headElements = () => ({ scripts: new Set(), styles: new Set(), links: new Set() });
pipeline.componentMetadata = () => {};
pipeline.componentMetadata = () => new Map;
return pipeline;
}