Skip to content

Commit

Permalink
fix(Astro global): parity with ctx.site (#10325)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
lilnasy authored Mar 7, 2024
1 parent fdd5bf2 commit f33cce8
Show file tree
Hide file tree
Showing 16 changed files with 229 additions and 303 deletions.
5 changes: 5 additions & 0 deletions .changeset/sour-apples-try.md
Original file line number Diff line number Diff line change
@@ -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.
2 changes: 1 addition & 1 deletion packages/astro/src/@types/astro.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion packages/astro/src/core/base-pipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
4 changes: 1 addition & 3 deletions packages/astro/src/core/build/generate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions packages/astro/src/core/compile/compile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
15 changes: 14 additions & 1 deletion packages/astro/src/core/errors/errors-data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
2 changes: 0 additions & 2 deletions packages/astro/src/core/middleware/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,6 @@ function createContext({
return (currentLocale ??= computeCurrentLocale(
route,
userDefinedLocales,
undefined,
undefined
));
},
url,
Expand Down
194 changes: 135 additions & 59 deletions packages/astro/src/core/render-context.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
import type {
APIContext,
AstroGlobal,
AstroGlobalPartial,
ComponentInstance,
MiddlewareHandler,
RouteData,
SSRResult,
} from '../@types/astro.js';
import {
computeCurrentLocale,
Expand All @@ -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(
Expand Down Expand Up @@ -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;
},
Expand All @@ -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);
}
}

/**
Expand All @@ -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'];
Expand Down
2 changes: 1 addition & 1 deletion packages/astro/src/core/render/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down
Loading

0 comments on commit f33cce8

Please sign in to comment.