Skip to content

Commit

Permalink
Add additional scoping for head buffering (#6152)
Browse files Browse the repository at this point in the history
* Add additional scoping for head buffering

* Add test for direct usage of nested component

* Add special scoping for Astro.scopes.render()

* Generate propagation map during the build

* Move to a maybeHead instruction

* Properly serialize for SSR

* More conservative scoping

* Maybe had should honor result._metadata.hasRenderedHead

* Properly type slots

* Allow template result to be passed

* Add changeset
  • Loading branch information
matthewp committed Feb 7, 2023
1 parent cee70f5 commit d1f5611
Show file tree
Hide file tree
Showing 42 changed files with 386 additions and 68 deletions.
10 changes: 10 additions & 0 deletions .changeset/pretty-bananas-own.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
'astro': patch
---

Fix MDX related head placement bugs

This fixes a variety of head content placement bugs (such as page `<link>`) related to MDX, especially when used in content collections. Issues fixed:

- Head content being placed in the body instead of the head.
- Head content missing when rendering an MDX component from within a nested Astro component.
2 changes: 1 addition & 1 deletion packages/astro/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@
"test:e2e:match": "playwright test -g"
},
"dependencies": {
"@astrojs/compiler": "^1.0.1",
"@astrojs/compiler": "^1.1.0",
"@astrojs/language-server": "^0.28.3",
"@astrojs/markdown-remark": "^2.0.1",
"@astrojs/telemetry": "^2.0.0",
Expand Down
3 changes: 2 additions & 1 deletion packages/astro/src/content/internal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { prependForwardSlash } from '../core/path.js';
import {
createComponent,
createHeadAndContent,
createScopedResult,
renderComponent,
renderScriptElement,
renderStyleElement,
Expand Down Expand Up @@ -169,7 +170,7 @@ async function render({

return createHeadAndContent(
unescapeHTML(styles + links + scripts) as any,
renderTemplate`${renderComponent(result, 'Content', mod.Content, props, slots)}`
renderTemplate`${renderComponent(createScopedResult(result), 'Content', mod.Content, props, slots)}`
);
},
propagation: 'self',
Expand Down
2 changes: 2 additions & 0 deletions packages/astro/src/core/app/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,12 @@ export function deserializeManifest(serializedManifest: SerializedSSRManifest):
}

const assets = new Set<string>(serializedManifest.assets);
const propagation = new Map(serializedManifest.propagation);

return {
...serializedManifest,
assets,
propagation,
routes,
};
}
1 change: 1 addition & 0 deletions packages/astro/src/core/app/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,7 @@ export class App {
request,
origin: url.origin,
pathname,
propagation: this.#manifest.propagation,
scripts,
links,
route: routeData,
Expand Down
6 changes: 5 additions & 1 deletion packages/astro/src/core/app/types.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import type { MarkdownRenderingOptions } from '@astrojs/markdown-remark';
import type {
ComponentInstance,
PropagationHint,
RouteData,
SerializedRouteData,
SSRLoadedRenderer,
SSRResult,
} from '../../@types/astro';

export type ComponentPath = string;
Expand Down Expand Up @@ -34,11 +36,13 @@ export interface SSRManifest {
renderers: SSRLoadedRenderer[];
entryModules: Record<string, string>;
assets: Set<string>;
propagation: SSRResult['propagation'];
}

export type SerializedSSRManifest = Omit<SSRManifest, 'routes' | 'assets'> & {
export type SerializedSSRManifest = Omit<SSRManifest, 'routes' | 'assets' | 'propagation'> & {
routes: SerializedRouteInfo[];
assets: string[];
propagation: readonly [string, PropagationHint][];
};

export type AdapterCreateExports<T = any> = (
Expand Down
1 change: 1 addition & 0 deletions packages/astro/src/core/build/generate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,7 @@ async function generatePath(
origin,
pathname,
request: createRequest({ url, headers: new Headers(), logging, ssr }),
propagation: internals.propagation,
scripts,
links,
route: pageData.route,
Expand Down
3 changes: 3 additions & 0 deletions packages/astro/src/core/build/internal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import type { PageBuildData, ViteID } from './types';
import { PageOptions } from '../../vite-plugin-astro/types';
import { prependForwardSlash, removeFileExtension } from '../path.js';
import { viteID } from '../util.js';
import { SSRResult } from '../../@types/astro';

export interface BuildInternals {
/**
Expand Down Expand Up @@ -66,6 +67,7 @@ export interface BuildInternals {
staticFiles: Set<string>;
// The SSR entry chunk. Kept in internals to share between ssr/client build steps
ssrEntryChunk?: OutputChunk;
propagation: SSRResult['propagation'];
}

/**
Expand Down Expand Up @@ -95,6 +97,7 @@ export function createBuildInternals(): BuildInternals {
discoveredClientOnlyComponents: new Set(),
discoveredScripts: new Set(),
staticFiles: new Set(),
propagation: new Map(),
};
}

Expand Down
2 changes: 2 additions & 0 deletions packages/astro/src/core/build/plugins/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,15 @@ import { pluginInternals } from './plugin-internals.js';
import { pluginPages } from './plugin-pages.js';
import { pluginPrerender } from './plugin-prerender.js';
import { pluginSSR } from './plugin-ssr.js';
import { astroHeadPropagationBuildPlugin } from '../../../vite-plugin-head-propagation/index.js';

export function registerAllPlugins({ internals, options, register }: AstroBuildPluginContainer) {
register(pluginAliasResolve(internals));
register(pluginAnalyzer(internals));
register(pluginInternals(internals));
register(pluginPages(options, internals));
register(pluginCSS(options, internals));
register(astroHeadPropagationBuildPlugin(options, internals));
register(pluginPrerender(options, internals));
register(astroConfigBuildPlugin(options, internals));
register(pluginHoistedScripts(options, internals));
Expand Down
1 change: 1 addition & 0 deletions packages/astro/src/core/build/plugins/plugin-ssr.ts
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ function buildManifest(
contentDir: getContentPaths(settings.config).contentDir,
},
pageMap: null as any,
propagation: Array.from(internals.propagation),
renderers: [],
entryModules,
assets: staticFiles.map((s) => settings.config.base + s),
Expand Down
1 change: 1 addition & 0 deletions packages/astro/src/core/compile/compile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ export async function compile({
sourcemap: 'both',
internalURL: 'astro/server/index.js',
astroGlobalArgs: JSON.stringify(astroConfig.site),
resultScopedSlot: true,
preprocessStyle: createStylePreprocessor({
filename,
viteConfig,
Expand Down
19 changes: 10 additions & 9 deletions packages/astro/src/core/render/result.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import type {
SSRLoadedRenderer,
SSRResult,
} from '../../@types/astro';
import { renderSlot, stringifyChunk } from '../../runtime/server/index.js';
import { renderSlot, stringifyChunk, ScopeFlags, createScopedResult, ComponentSlots } from '../../runtime/server/index.js';
import { renderJSX } from '../../runtime/server/jsx.js';
import { AstroCookies } from '../cookies/index.js';
import { AstroError, AstroErrorData } from '../errors/index.js';
Expand Down Expand Up @@ -55,10 +55,10 @@ function getFunctionExpression(slot: any) {

class Slots {
#result: SSRResult;
#slots: Record<string, any> | null;
#slots: ComponentSlots | null;
#loggingOpts: LogOptions;

constructor(result: SSRResult, slots: Record<string, any> | null, logging: LogOptions) {
constructor(result: SSRResult, slots: ComponentSlots | null, logging: LogOptions) {
this.#result = result;
this.#slots = slots;
this.#loggingOpts = logging;
Expand Down Expand Up @@ -89,6 +89,7 @@ class Slots {
public async render(name: string, args: any[] = []) {
if (!this.#slots || !this.has(name)) return;

const scoped = createScopedResult(this.#result, ScopeFlags.RenderSlot);
if (!Array.isArray(args)) {
warn(
this.#loggingOpts,
Expand All @@ -97,26 +98,26 @@ class Slots {
);
} else if (args.length > 0) {
const slotValue = this.#slots[name];
const component = typeof slotValue === 'function' ? await slotValue() : await slotValue;
const component = typeof slotValue === 'function' ? await slotValue(scoped) : await slotValue;

// Astro
const expression = getFunctionExpression(component);
if (expression) {
const slot = expression(...args);
return await renderSlot(this.#result, slot).then((res) =>
const slot = () => expression(...args);
return await renderSlot(scoped, slot).then((res) =>
res != null ? String(res) : res
);
}
// JSX
if (typeof component === 'function') {
return await renderJSX(this.#result, component(...args)).then((res) =>
return await renderJSX(scoped, (component as any)(...args)).then((res) =>
res != null ? String(res) : res
);
}
}

const content = await renderSlot(this.#result, this.#slots[name]);
const outHTML = stringifyChunk(this.#result, content);
const content = await renderSlot(scoped, this.#slots[name]);
const outHTML = stringifyChunk(scoped, content);

return outHTML;
}
Expand Down
7 changes: 5 additions & 2 deletions packages/astro/src/runtime/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,13 @@ export { escapeHTML, HTMLBytes, HTMLString, markHTMLString, unescapeHTML } from
export { renderJSX } from './jsx.js';
export {
addAttribute,
addScopeFlag,
createHeadAndContent,
createScopedResult,
defineScriptVars,
Fragment,
maybeRenderHead,
removeScopeFlag,
renderAstroTemplateResult as renderAstroComponent,
renderComponent,
renderComponentToIterable,
Expand All @@ -23,15 +26,15 @@ export {
renderTemplate,
renderToString,
renderUniqueStylesheet,
ScopeFlags,
stringifyChunk,
voidElementNames,
} from './render/index.js';
export type {
AstroComponentFactory,
AstroComponentInstance,
AstroComponentSlots,
AstroComponentSlotsWithValues,
RenderInstruction,
ComponentSlots
} from './render/index.js';

import { markHTMLString } from './escape.js';
Expand Down
7 changes: 4 additions & 3 deletions packages/astro/src/runtime/server/jsx.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
} from './index.js';
import { HTMLParts } from './render/common.js';
import type { ComponentIterable } from './render/component';
import { ScopeFlags } from './render/util.js';
import { createScopedResult, ScopeFlags } from './render/scope.js';

const ClientOnlyPlaceholder = 'astro-client-only';

Expand Down Expand Up @@ -95,8 +95,9 @@ Did you forget to import the component or is it possible there is a typo?`);
props[key] = value;
}
}
result.scope |= ScopeFlags.JSX;
return markHTMLString(await renderToString(result, vnode.type as any, props, slots));
const scoped = createScopedResult(result, ScopeFlags.JSX);
const html = markHTMLString(await renderToString(scoped, vnode.type as any, props, slots));
return html;
}
case !vnode.type && (vnode.type as any) !== 0:
return '';
Expand Down
6 changes: 3 additions & 3 deletions packages/astro/src/runtime/server/render/astro/factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import type { HeadAndContent } from './head-and-content';
import type { RenderTemplateResult } from './render-template';

import { HTMLParts } from '../common.js';
import { ScopeFlags } from '../util.js';
import { addScopeFlag, createScopedResult, ScopeFlags } from '../scope.js';
import { isHeadAndContent } from './head-and-content.js';
import { renderAstroTemplateResult } from './render-template.js';

Expand All @@ -28,8 +28,8 @@ export async function renderToString(
props: any,
children: any
): Promise<string> {
result.scope |= ScopeFlags.Astro;
const factoryResult = await componentFactory(result, props, children);
const scoped = createScopedResult(result, ScopeFlags.Astro);
const factoryResult = await componentFactory(scoped, props, children);

if (factoryResult instanceof Response) {
const response = factoryResult;
Expand Down
2 changes: 1 addition & 1 deletion packages/astro/src/runtime/server/render/astro/index.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
export type { AstroComponentFactory } from './factory';
export { isAstroComponentFactory, renderToString } from './factory.js';
export { createHeadAndContent, isHeadAndContent } from './head-and-content.js';
export type { AstroComponentInstance, ComponentSlots, ComponentSlotsWithValues } from './instance';
export type { AstroComponentInstance } from './instance';
export { createAstroComponentInstance, isAstroComponentInstance } from './instance.js';
export {
isRenderTemplateResult,
Expand Down
18 changes: 9 additions & 9 deletions packages/astro/src/runtime/server/render/astro/instance.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,15 @@
import type { SSRResult } from '../../../../@types/astro';
import type { AstroComponentFactory, AstroFactoryReturnValue } from './factory.js';
import type { renderTemplate } from './render-template.js';
import type { ComponentSlots } from '../slot.js';

import { HydrationDirectiveProps } from '../../hydration.js';
import { isPromise } from '../../util.js';
import { renderChild } from '../any.js';
import { isAPropagatingComponent } from './factory.js';
import { isHeadAndContent } from './head-and-content.js';
import { createScopedResult, ScopeFlags } from '../scope.js';

type ComponentProps = Record<string | number, any>;
type ComponentSlotValue = () => ReturnType<typeof renderTemplate>;
export type ComponentSlots = Record<string, ComponentSlotValue>;
export type ComponentSlotsWithValues = Record<string, ReturnType<ComponentSlotValue>>;

const astroComponentInstanceSym = Symbol.for('astro.componentInstance');

Expand All @@ -20,7 +18,7 @@ export class AstroComponentInstance {

private readonly result: SSRResult;
private readonly props: ComponentProps;
private readonly slotValues: ComponentSlotsWithValues;
private readonly slotValues: ComponentSlots;
private readonly factory: AstroComponentFactory;
private returnValue: ReturnType<AstroComponentFactory> | undefined;
constructor(
Expand All @@ -33,19 +31,21 @@ export class AstroComponentInstance {
this.props = props;
this.factory = factory;
this.slotValues = {};
const scoped = createScopedResult(result, ScopeFlags.Slot);
for (const name in slots) {
this.slotValues[name] = slots[name]();
const value = slots[name](scoped);
this.slotValues[name] = () => value;
}
}

async init() {
this.returnValue = this.factory(this.result, this.props, this.slotValues);
async init(result: SSRResult) {
this.returnValue = this.factory(result, this.props, this.slotValues);
return this.returnValue;
}

async *render() {
if (this.returnValue === undefined) {
await this.init();
await this.init(this.result);
}

let value: AstroFactoryReturnValue | undefined = this.returnValue;
Expand Down
25 changes: 25 additions & 0 deletions packages/astro/src/runtime/server/render/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
} from '../scripts.js';
import { renderAllHeadContent } from './head.js';
import { isSlotString, type SlotString } from './slot.js';
import { ScopeFlags } from './scope.js';

export const Fragment = Symbol.for('astro:fragment');
export const Renderer = Symbol.for('astro:renderer');
Expand Down Expand Up @@ -48,6 +49,30 @@ export function stringifyChunk(result: SSRResult, chunk: string | SlotString | R
}
return renderAllHeadContent(result);
}
case 'maybe-head': {
if (result._metadata.hasRenderedHead) {
return '';
}

const scope = instruction.scope;
switch (scope) {
// JSX with an Astro slot
case ScopeFlags.JSX | ScopeFlags.Slot | ScopeFlags.Astro:
case ScopeFlags.JSX | ScopeFlags.Astro | ScopeFlags.HeadBuffer:
case ScopeFlags.JSX | ScopeFlags.Slot | ScopeFlags.Astro | ScopeFlags.HeadBuffer: {
return '';
}

// Astro.slots.render('default') should never render head content.
case ScopeFlags.RenderSlot | ScopeFlags.Astro:
case ScopeFlags.RenderSlot | ScopeFlags.Astro | ScopeFlags.JSX:
case ScopeFlags.RenderSlot | ScopeFlags.Astro | ScopeFlags.JSX | ScopeFlags.HeadBuffer: {
return '';
}
}

return renderAllHeadContent(result);
}
}
} else {
if (isSlotString(chunk as string)) {
Expand Down
Loading

0 comments on commit d1f5611

Please sign in to comment.