feat(next): TODOs (#11987)

* feat(next): TODOs

* fix: remove todos

* Updates tests to not assign locals

* Changesets

* Update .changeset/cool-mangos-shop.md

Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>

* Update .changeset/blue-sloths-stare.md

Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>

* Update .changeset/blue-sloths-stare.md

Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>

* Update .changeset/blue-sloths-stare.md

---------

Co-authored-by: Erika <3019731+Princesseuh@users.noreply.github.com>
Co-authored-by: Matthew Phillips <matthew@skypack.dev>
Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>

.changeset/blue-sloths-stare.md

@@ -0,0 +1,14 @@
+---
+'astro': patch
+---
+
+`render()` signature now takes `renderOptions` as 2nd argument
+
+The signature for `app.render()` has changed, and the second argument is now an options object called `renderOptions` with more options for customizing rendering.
+
+The `renderOptions` are:
+
+- `addCookieHeader`: Determines whether Astro will set the `Set-Cookie` header, otherwise the adapter is expected to do so itself.
+- `clientAddress`: The client IP address used to set `Astro.clientAddress`.
+- `locals`: An object of locals that's set to `Astro.locals`.
+- `routeData`: An object specifying the route to use.

.changeset/cool-mangos-shop.md

@@ -0,0 +1,25 @@
+---
+'astro': major
+---
+
+The `locals` object can no longer be overridden
+
+Middleware, API endpoints, and pages can no longer override the `locals` object in its entirety. You can still append values onto the object, but you can not replace the entire object and delete its existing values.
+
+If you were previously overwriting like so:
+
+```js
+ctx.locals = {
+  one: 1,
+  two: 2
+}
+```
+
+This can be changed to an assignment on the existing object instead:
+
+```js
+Object.assign(ctx.locals, {
+  one: 1,
+  two: 2
+})
+```

packages/astro/src/assets/utils/node/emitAsset.ts

@@ -12,7 +12,7 @@ export async function emitESMImage(
 	id: string | undefined,
 	/** @deprecated */
 	_watchMode: boolean,
-	// FIX: in Astro 5, this function should not be passed in dev mode at all.
+	// FIX: in Astro 6, this function should not be passed in dev mode at all.
 	// Or rethink the API so that a function that throws isn't passed through.
 	fileEmitter?: FileEmitter,
 ): Promise<ImageMetadata | undefined> {

packages/astro/src/content/runtime.ts

@@ -228,7 +228,6 @@ export function createGetDataEntryById({
 
 		const lazyImport = await getEntryImport(collection, id);
 
-		// TODO: AstroError
 		if (!lazyImport) throw new Error(`Entry ${collection} → ${id} was not found.`);
 		const entry = await lazyImport();
 

packages/astro/src/core/app/index.ts

@@ -245,48 +245,17 @@ export class App {
 		return pathname;
 	}
 
-	async render(request: Request, options?: RenderOptions): Promise<Response>;
-	/**
-	 * @deprecated Instead of passing `RouteData` and locals individually, pass an object with `routeData` and `locals` properties.
-	 * See https://github.com/withastro/astro/pull/9199 for more information.
-	 */
-	async render(request: Request, routeData?: RouteData, locals?: object): Promise<Response>;
-	async render(
-		request: Request,
-		routeDataOrOptions?: RouteData | RenderOptions,
-		maybeLocals?: object,
-	): Promise<Response> {
+	async render(request: Request, renderOptions?: RenderOptions): Promise<Response> {
 		let routeData: RouteData | undefined;
 		let locals: object | undefined;
 		let clientAddress: string | undefined;
 		let addCookieHeader: boolean | undefined;
 
-		if (
-			routeDataOrOptions &&
-			('addCookieHeader' in routeDataOrOptions ||
-				'clientAddress' in routeDataOrOptions ||
-				'locals' in routeDataOrOptions ||
-				'routeData' in routeDataOrOptions)
-		) {
-			if ('addCookieHeader' in routeDataOrOptions) {
-				addCookieHeader = routeDataOrOptions.addCookieHeader;
-			}
-			if ('clientAddress' in routeDataOrOptions) {
-				clientAddress = routeDataOrOptions.clientAddress;
-			}
-			if ('routeData' in routeDataOrOptions) {
-				routeData = routeDataOrOptions.routeData;
-			}
-			if ('locals' in routeDataOrOptions) {
-				locals = routeDataOrOptions.locals;
-			}
-		} else {
-			routeData = routeDataOrOptions as RouteData | undefined;
-			locals = maybeLocals;
-			if (routeDataOrOptions || locals) {
-				this.#logRenderOptionsDeprecationWarning();
-			}
-		}
+		addCookieHeader = renderOptions?.addCookieHeader;
+		clientAddress = renderOptions?.clientAddress;
+		routeData = renderOptions?.routeData;
+		locals = renderOptions?.locals;
+
 		if (routeData) {
 			this.#logger.debug(
 				'router',
@@ -367,15 +336,6 @@ export class App {
 		return response;
 	}
 
-	#logRenderOptionsDeprecationWarning() {
-		if (this.#renderOptionsDeprecationWarningShown) return;
-		this.#logger.warn(
-			'deprecated',
-			`The adapter ${this.#manifest.adapterName} is using a deprecated signature of the 'app.render()' method. From Astro 4.0, locals and routeData are provided as properties on an optional object to this method. Using the old signature will cause an error in Astro 5.0. See https://github.com/withastro/astro/pull/9199 for more information.`,
-		);
-		this.#renderOptionsDeprecationWarningShown = true;
-	}
-
 	setCookieHeaders(response: Response) {
 		return getSetCookiesFromResponse(response);
 	}

packages/astro/src/core/build/internal.ts

@@ -228,47 +228,6 @@ export function getPageData(
 	return undefined;
 }
 
-// TODO: Should be removed in the future. (Astro 5?)
-/**
- * Map internals.pagesByKeys to a new map with the public key instead of the internal key.
- * This function is only used to avoid breaking changes in the Integrations API, after we changed the way
- * we identify pages, from the entrypoint component to an internal key.
- * If the page component is unique -> the public key is the component path. (old behavior)
- * If the page component is shared -> the public key is the internal key. (new behavior)
- * The new behavior on shared entrypoint it's not a breaking change, because it was not supported before.
- * @param pagesByKeys A map of all page data by their internal key
- */
-export function getPageDatasWithPublicKey(
-	pagesByKeys: Map<string, PageBuildData>,
-): Map<string, PageBuildData> {
-	// Create a map to store the pages with the public key, mimicking internal.pagesByKeys
-	const pagesWithPublicKey = new Map<string, PageBuildData>();
-
-	const pagesByComponentsArray = Array.from(pagesByKeys.values()).map((pageData) => {
-		return { component: pageData.component, pageData: pageData };
-	});
-
-	// Get pages with unique component, and set the public key to the component.
-	const pagesWithUniqueComponent = pagesByComponentsArray.filter((page) => {
-		return pagesByComponentsArray.filter((p) => p.component === page.component).length === 1;
-	});
-
-	pagesWithUniqueComponent.forEach((page) => {
-		pagesWithPublicKey.set(page.component, page.pageData);
-	});
-
-	// Get pages with shared component, and set the public key to the internal key.
-	const pagesWithSharedComponent = pagesByComponentsArray.filter((page) => {
-		return pagesByComponentsArray.filter((p) => p.component === page.component).length > 1;
-	});
-
-	pagesWithSharedComponent.forEach((page) => {
-		pagesWithPublicKey.set(page.pageData.key, page.pageData);
-	});
-
-	return pagesWithPublicKey;
-}
-
 export function getPageDataByViteID(
 	internals: BuildInternals,
 	viteid: ViteID,

packages/astro/src/core/build/static-build.ts

@@ -11,11 +11,7 @@ import {
 	hasAnyContentFlag,
 	reverseSymlink,
 } from '../../content/utils.js';
-import {
-	type BuildInternals,
-	createBuildInternals,
-	getPageDatasWithPublicKey,
-} from '../../core/build/internal.js';
+import { type BuildInternals, createBuildInternals } from '../../core/build/internal.js';
 import { emptyDir, removeEmptyDirs } from '../../core/fs/index.js';
 import { appendForwardSlash, prependForwardSlash, removeFileExtension } from '../../core/path.js';
 import { runHookBuildSetup } from '../../integrations/hooks.js';
@@ -283,7 +279,7 @@ async function ssrBuild(
 
 	const updatedViteBuildConfig = await runHookBuildSetup({
 		config: settings.config,
-		pages: getPageDatasWithPublicKey(internals.pagesByKeys),
+		pages: internals.pagesByKeys,
 		vite: viteBuildConfig,
 		target: 'server',
 		logger: opts.logger,
@@ -344,7 +340,7 @@ async function clientBuild(
 
 	await runHookBuildSetup({
 		config: settings.config,
-		pages: getPageDatasWithPublicKey(internals.pagesByKeys),
+		pages: internals.pagesByKeys,
 		vite: viteBuildConfig,
 		target: 'client',
 		logger: opts.logger,

packages/astro/src/core/config/schema.ts

@@ -333,17 +333,6 @@ export const AstroConfigSchema = z.object({
 					transformers: z
 						.custom<ShikiTransformer>()
 						.array()
-						.transform((transformers) => {
-							for (const transformer of transformers) {
-								// When updating shikiji to shiki, the `token` property was renamed to `span`.
-								// We apply the compat here for now until the next Astro major.
-								// TODO: Remove this in Astro 5.0
-								if ((transformer as any).token && !('span' in transformer)) {
-									transformer.span = (transformer as any).token;
-								}
-							}
-							return transformers;
-						})
 						.default(ASTRO_CONFIG_DEFAULTS.markdown.shikiConfig.transformers!),
 				})
 				.default({}),

packages/astro/src/core/errors/errors-data.ts

@@ -843,6 +843,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 `locals` field on the Astro global or context.
+ */
+export const LocalsReassigned = {
+	name: 'LocalsReassigned',
+	title: '`locals` must not be reassigned.',
+	message: '`locals` can not be assigned directly.',
+	hint: 'Set a `locals` property instead.',
+} satisfies ErrorData;
+
+
 /**
  * @docs
  * @description

packages/astro/src/core/middleware/index.ts

@@ -98,13 +98,8 @@ function createContext({
 			}
 			return locals;
 		},
-		// We define a custom property, so we can check the value passed to locals
-		set locals(val) {
-			if (typeof val !== 'object') {
-				throw new AstroError(AstroErrorData.LocalsNotAnObject);
-			} else {
-				Reflect.set(request, clientLocalsSymbol, val);
-			}
+		set locals(_) {
+			throw new AstroError(AstroErrorData.LocalsReassigned);
 		},
 	};
 	return Object.assign(context, {

packages/astro/src/core/render-context.ts

@@ -19,7 +19,6 @@ import {
 	REWRITE_DIRECTIVE_HEADER_VALUE,
 	ROUTE_TYPE_HEADER,
 	clientAddressSymbol,
-	clientLocalsSymbol,
 	responseSentSymbol,
 } from './constants.js';
 import { AstroCookies, attachCookiesToResponse } from './cookies/index.js';
@@ -270,16 +269,8 @@ export class RenderContext {
 			get locals() {
 				return renderContext.locals;
 			},
-			// TODO(breaking): disallow replacing the locals object
-			set locals(val) {
-				if (typeof val !== 'object') {
-					throw new AstroError(AstroErrorData.LocalsNotAnObject);
-				} else {
-					renderContext.locals = val;
-					// we also put it on the original Request object,
-					// where the adapter might be expecting to read it after the response.
-					Reflect.set(this.request, clientLocalsSymbol, val);
-				}
+			set locals(_) {
+				throw new AstroError(AstroErrorData.LocalsReassigned);
 			},
 			params,
 			get preferredLocale() {

packages/astro/src/runtime/client/dev-toolbar/apps/astro.ts

@@ -1,4 +1,7 @@
-import type { DevToolbarApp, DevToolbarMetadata } from '../../../../types/public/toolbar.js';
+import type {
+	DevToolbarMetadata,
+	ResolvedDevToolbarApp,
+} from '../../../../types/public/toolbar.js';
 import { type Icon, isDefinedIcon } from '../ui-library/icons.js';
 import { colorForIntegration, iconForIntegration } from './utils/icons.js';
 import {
@@ -460,4 +463,4 @@ export default {
 			integrationList.append(fragment);
 		}
 	},
-} satisfies DevToolbarApp;
+} satisfies ResolvedDevToolbarApp;

packages/astro/src/runtime/client/dev-toolbar/apps/audit/index.ts

@@ -1,4 +1,4 @@
-import type { DevToolbarApp } from '../../../../../types/public/toolbar.js';
+import type { ResolvedDevToolbarApp } from '../../../../../types/public/toolbar.js';
 import { settings } from '../../settings.js';
 import type { DevToolbarHighlight } from '../../ui-library/highlight.js';
 import { positionHighlight } from '../utils/highlight.js';
@@ -219,4 +219,4 @@ export default {
 			});
 		}
 	},
-} satisfies DevToolbarApp;
+} satisfies ResolvedDevToolbarApp;

packages/astro/src/runtime/client/dev-toolbar/apps/settings.ts

@@ -1,4 +1,4 @@
-import type { DevToolbarApp } from '../../../../types/public/toolbar.js';
+import type { ResolvedDevToolbarApp } from '../../../../types/public/toolbar.js';
 import { type Settings, settings } from '../settings.js';
 import { isValidPlacement, placements } from '../ui-library/window.js';
 import {
@@ -212,4 +212,4 @@ export default {
 			}
 		}
 	},
-} satisfies DevToolbarApp;
+} satisfies ResolvedDevToolbarApp;

packages/astro/src/runtime/client/dev-toolbar/apps/xray.ts

@@ -1,5 +1,8 @@
 import { escape as escapeHTML } from 'html-escaper';
-import type { DevToolbarApp, DevToolbarMetadata } from '../../../../types/public/toolbar.js';
+import type {
+	DevToolbarMetadata,
+	ResolvedDevToolbarApp,
+} from '../../../../types/public/toolbar.js';
 import type { DevToolbarHighlight } from '../ui-library/highlight.js';
 import {
 	attachTooltipToHighlight,
@@ -177,4 +180,4 @@ export default {
 			return tooltip;
 		}
 	},
-} satisfies DevToolbarApp;
+} satisfies ResolvedDevToolbarApp;

packages/astro/src/runtime/client/dev-toolbar/entrypoint.ts

@@ -51,20 +51,6 @@ document.addEventListener('DOMContentLoaded', async () => {
 	customElements.define('astro-dev-toolbar-select', DevToolbarSelect);
 	customElements.define('astro-dev-toolbar-radio-checkbox', DevToolbarRadioCheckbox);
 
-	// Add deprecated names
-	// TODO: Remove in Astro 5.0
-	const deprecated = (Parent: any) => class extends Parent {};
-	customElements.define('astro-dev-overlay', deprecated(AstroDevToolbar));
-	customElements.define('astro-dev-overlay-window', deprecated(DevToolbarWindow));
-	customElements.define('astro-dev-overlay-plugin-canvas', deprecated(DevToolbarCanvas));
-	customElements.define('astro-dev-overlay-tooltip', deprecated(DevToolbarTooltip));
-	customElements.define('astro-dev-overlay-highlight', deprecated(DevToolbarHighlight));
-	customElements.define('astro-dev-overlay-card', deprecated(DevToolbarCard));
-	customElements.define('astro-dev-overlay-toggle', deprecated(DevToolbarToggle));
-	customElements.define('astro-dev-overlay-button', deprecated(DevToolbarButton));
-	customElements.define('astro-dev-overlay-badge', deprecated(DevToolbarBadge));
-	customElements.define('astro-dev-overlay-icon', deprecated(DevToolbarIcon));
-
 	overlay = document.createElement('astro-dev-toolbar');
 
 	const notificationLevels = ['error', 'warning', 'info'] as const;
@@ -121,9 +107,6 @@ document.addEventListener('DOMContentLoaded', async () => {
 		};
 
 		eventTarget.addEventListener('toggle-app', onToggleApp);
-		// Deprecated
-		// TODO: Remove in Astro 5.0
-		eventTarget.addEventListener('toggle-plugin', onToggleApp);
 
 		return app;
 	};

packages/astro/src/runtime/client/dev-toolbar/settings.ts

@@ -18,13 +18,6 @@ function getSettings() {
 	let _settings: Settings = { ...defaultSettings };
 	const toolbarSettings = localStorage.getItem('astro:dev-toolbar:settings');
 
-	// TODO: Remove in Astro 5.0
-	const oldSettings = localStorage.getItem('astro:dev-overlay:settings');
-	if (oldSettings && !toolbarSettings) {
-		localStorage.setItem('astro:dev-toolbar:settings', oldSettings);
-		localStorage.removeItem('astro:dev-overlay:settings');
-	}
-
 	if (toolbarSettings) {
 		_settings = { ..._settings, ...JSON.parse(toolbarSettings) };
 	}

packages/astro/src/runtime/client/dev-toolbar/toolbar.ts

@@ -16,8 +16,6 @@ export type DevToolbarApp = DevToolbarAppDefinition & {
 	eventTarget: ToolbarAppEventTarget;
 };
 const WS_EVENT_NAME = 'astro-dev-toolbar';
-// TODO: Remove in Astro 5.0
-const WS_EVENT_NAME_DEPRECATED = 'astro-dev-overlay';
 
 const HOVER_DELAY = 2 * 1000;
 const DEVBAR_HITBOX_ABOVE = 42;
@@ -396,8 +394,6 @@ export class AstroDevToolbar extends HTMLElement {
 
 			if (import.meta.hot) {
 				import.meta.hot.send(`${WS_EVENT_NAME}:${app.id}:initialized`);
-				// TODO: Remove in Astro 5.0
-				import.meta.hot.send(`${WS_EVENT_NAME_DEPRECATED}:${app.id}:initialized`);
 			}
 		} catch (e) {
 			console.error(`Failed to init app ${app.id}, error: ${e}`);
@@ -501,28 +497,16 @@ export class AstroDevToolbar extends HTMLElement {
 			appCanvas.removeAttribute('data-active');
 		}
 
-		[
-			'app-toggled',
-			// Deprecated
-			// TODO: Remove in Astro 5.0
-			'plugin-toggled',
-		].forEach((eventName) => {
 		app.eventTarget.dispatchEvent(
-				new CustomEvent(eventName, {
+			new CustomEvent('app-toggled', {
 				detail: {
 					state: app.active,
 					app,
 				},
 			}),
 		);
-		});
 
-		if (import.meta.hot) {
-			import.meta.hot.send(`${WS_EVENT_NAME}:${app.id}:toggled`, { state: app.active });
-			import.meta.hot.send(`${WS_EVENT_NAME_DEPRECATED}:${app.id}:toggled`, {
-				state: app.active,
-			});
-		}
+		import.meta.hot?.send(`${WS_EVENT_NAME}:${app.id}:toggled`, { state: app.active });
 
 		return true;
 	}

packages/astro/src/runtime/client/dev-toolbar/ui-library/button.ts

@@ -131,8 +131,6 @@ export class DevToolbarButton extends HTMLElement {
 					cursor: pointer;
 				}
 
-				/* TODO: Remove "astro-dev-overlay-icon" in Astro 5.0 */
-				::slotted(astro-dev-overlay-icon),
 				::slotted(astro-dev-toolbar-icon) {
 					display: inline-block;
 					height: 1em;

packages/astro/src/types/public/integrations.ts

@@ -174,8 +174,7 @@ export interface BaseIntegrationHooks {
 		injectScript: (stage: InjectedScriptStage, content: string) => void;
 		injectRoute: (injectRoute: InjectedRoute) => void;
 		addClientDirective: (directive: ClientDirectiveConfig) => void;
-		// TODO: Deprecate the `string` overload once a few apps have been migrated to the new API.
-		addDevToolbarApp: (entrypoint: DevToolbarAppEntry | string) => void;
+		addDevToolbarApp: (entrypoint: DevToolbarAppEntry) => void;
 		addMiddleware: (mid: AstroIntegrationMiddleware) => void;
 		logger: AstroIntegrationLogger;
 	}) => void | Promise<void>;

packages/astro/src/types/public/toolbar.ts

@@ -50,30 +50,6 @@ export type DevToolbarAppEntry = DevToolbarAppMeta & {
 
 // Public API for the dev toolbar
 export type DevToolbarApp = {
-	/**
-	 * @deprecated The `id`, `name`, and `icon` properties should now be defined when using `addDevToolbarApp`.
-	 *
-	 * Ex: `addDevToolbarApp({ id: 'my-app', name: 'My App', icon: '🚀', entrypoint: '/path/to/app' })`
-	 *
-	 * In the future, putting these properties directly on the app object will be removed.
-	 */
-	id?: string;
-	/**
-	 * @deprecated The `id`, `name`, and `icon` properties should now be defined when using `addDevToolbarApp`.
-	 *
-	 * Ex: `addDevToolbarApp({ id: 'my-app', name: 'My App', icon: '🚀', entrypoint: '/path/to/app' })`
-	 *
-	 * In the future, putting these properties directly on the app object will be removed.
-	 */
-	name?: string;
-	/**
-	 * @deprecated The `id`, `name`, and `icon` properties should now be defined when using `addDevToolbarApp`.
-	 *
-	 * Ex: `addDevToolbarApp({ id: 'my-app', name: 'My App', icon: '🚀', entrypoint: '/path/to/app' })`
-	 *
-	 * In the future, putting these properties directly on the app object will be removed.
-	 */
-	icon?: Icon;
 	init?(
 		canvas: ShadowRoot,
 		app: ToolbarAppEventTarget,
@@ -83,7 +59,7 @@ export type DevToolbarApp = {
 };
 
 // An app that has been loaded and as such contain all of its properties
-export type ResolvedDevToolbarApp = DevToolbarAppMeta & Omit<DevToolbarApp, 'id' | 'name' | 'icon'>;
+export type ResolvedDevToolbarApp = DevToolbarAppMeta & DevToolbarApp;
 
 export type DevToolbarMetadata = Window &
 	typeof globalThis & {

packages/astro/test/fixtures/astro-global/src/middleware.js

@@ -1,9 +1,9 @@
 
 
 export function onRequest(ctx, next) {
-	ctx.locals = {
+	Object.assign(ctx.locals, {
 		localsPattern: ctx.routePattern,
 		localsPrerendered: ctx.isPrerendered
-	};
+	});
 	return next()
 }

packages/astro/test/fixtures/middleware space/src/middleware.js

@@ -47,9 +47,9 @@ const first = defineMiddleware(async (context, next) => {
 			context.cookies.set('foo', 'bar');
 		}
 
-		context.locals = {
+		Object.assign(context.locals, {
 			name: 'bar',
-		};
+		});
 	}
 	return await next();
 });
@@ -65,9 +65,9 @@ const second = defineMiddleware(async (context, next) => {
 
 const third = defineMiddleware(async (context, next) => {
 	if (context.request.url.includes('/broken-locals')) {
-		context.locals = {
+		Object.assign(context.locals, {
 			fn() {},
-		};
+		});
 	} else if (context.request.url.includes('/does-nothing')) {
 		return undefined;
 	}

packages/astro/test/fixtures/rewrite-custom-404/src/middleware.js

@@ -2,9 +2,9 @@
 
 export const onRequest = async (context, next) => {
 		if (context.url.pathname.startsWith("/404") || context.url.pathname.startsWith("/500")) {
-			context.locals = {
+			Object.assign(context.locals, {
 				interjected: "Interjected"
-			}
+			});
 		}
 		return  await next();
 }