Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(next): TODOs #11987

Merged
merged 11 commits into from
Sep 13, 2024
14 changes: 14 additions & 0 deletions .changeset/blue-sloths-stare.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
---
'astro': patch
---

App class now accepts renderOptions
Copy link
Member

@sarah11918 sarah11918 Sep 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just checking, does this need updating in the docs here: https://docs.astro.build/en/reference/adapter-reference/#apprenderrequest-request-options-renderoptions and we should mention it in the upgrade guide as a Breaking Change

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
App class now accepts renderOptions
Adds a new `renderOptions` object for `App/class`

Not sure about the syntax for App class, but it likely needs something!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

App is a class, this is for the render method. I'll update to something.

matthewp marked this conversation as resolved.
Show resolved Hide resolved

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.
25 changes: 25 additions & 0 deletions .changeset/cool-mangos-shop.md
Original file line number Diff line number Diff line change
@@ -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
})
```
2 changes: 1 addition & 1 deletion packages/astro/src/assets/utils/node/emitAsset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we won't get to this right now, it's.. tough

// Or rethink the API so that a function that throws isn't passed through.
fileEmitter?: FileEmitter,
): Promise<ImageMetadata | undefined> {
Expand Down
1 change: 0 additions & 1 deletion packages/astro/src/content/runtime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,6 @@ export function createGetDataEntryById({

const lazyImport = await getEntryImport(collection, id);

// TODO: AstroError
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will be removed in Astro 6.0, not worth doing it now

if (!lazyImport) throw new Error(`Entry ${collection} → ${id} was not found.`);
const entry = await lazyImport();

Expand Down
52 changes: 6 additions & 46 deletions packages/astro/src/core/app/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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);
}
Expand Down
41 changes: 0 additions & 41 deletions packages/astro/src/core/build/internal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
10 changes: 3 additions & 7 deletions packages/astro/src/core/build/static-build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
11 changes: 0 additions & 11 deletions packages/astro/src/core/config/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({}),
Expand Down
13 changes: 13 additions & 0 deletions packages/astro/src/core/errors/errors-data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 2 additions & 7 deletions packages/astro/src/core/middleware/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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, {
Expand Down
13 changes: 2 additions & 11 deletions packages/astro/src/core/render-context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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() {
Expand Down
7 changes: 5 additions & 2 deletions packages/astro/src/runtime/client/dev-toolbar/apps/astro.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -460,4 +463,4 @@ export default {
integrationList.append(fragment);
}
},
} satisfies DevToolbarApp;
} satisfies ResolvedDevToolbarApp;
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -219,4 +219,4 @@ export default {
});
}
},
} satisfies DevToolbarApp;
} satisfies ResolvedDevToolbarApp;
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -212,4 +212,4 @@ export default {
}
}
},
} satisfies DevToolbarApp;
} satisfies ResolvedDevToolbarApp;
7 changes: 5 additions & 2 deletions packages/astro/src/runtime/client/dev-toolbar/apps/xray.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -177,4 +180,4 @@ export default {
return tooltip;
}
},
} satisfies DevToolbarApp;
} satisfies ResolvedDevToolbarApp;
17 changes: 0 additions & 17 deletions packages/astro/src/runtime/client/dev-toolbar/entrypoint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
};
Expand Down
7 changes: 0 additions & 7 deletions packages/astro/src/runtime/client/dev-toolbar/settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) };
}
Expand Down
Loading
Loading