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

Add additional scoping for head buffering #6152

Merged
merged 11 commits into from
Feb 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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