diff options
13 files changed, 165 insertions, 43 deletions
diff --git a/.changeset/smart-elephants-check.md b/.changeset/smart-elephants-check.md new file mode 100644 index 000000000..0096e0314 --- /dev/null +++ b/.changeset/smart-elephants-check.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Hoist hydration scripts out of slot templates diff --git a/packages/astro/src/runtime/server/render/any.ts b/packages/astro/src/runtime/server/render/any.ts index 454ed700d..9d5593873 100644 --- a/packages/astro/src/runtime/server/render/any.ts +++ b/packages/astro/src/runtime/server/render/any.ts @@ -1,10 +1,15 @@ import { escapeHTML, HTMLString, markHTMLString } from '../escape.js'; import { AstroComponent, renderAstroComponent } from './astro.js'; -import { stringifyChunk } from './common.js'; +import { SlotString } from './slot.js'; export async function* renderChild(child: any): AsyncIterable<any> { child = await child; - if (child instanceof HTMLString) { + if(child instanceof SlotString) { + if(child.instructions) { + yield * child.instructions; + } + yield child; + } else if (child instanceof HTMLString) { yield child; } else if (Array.isArray(child)) { for (const value of child) { @@ -38,19 +43,3 @@ export async function* renderChild(child: any): AsyncIterable<any> { yield child; } } - -export async function renderSlot(result: any, slotted: string, fallback?: any): Promise<string> { - if (slotted) { - let iterator = renderChild(slotted); - let content = ''; - for await (const chunk of iterator) { - if ((chunk as any).type === 'directive') { - content += stringifyChunk(result, chunk); - } else { - content += chunk; - } - } - return markHTMLString(content); - } - return fallback; -} diff --git a/packages/astro/src/runtime/server/render/component.ts b/packages/astro/src/runtime/server/render/component.ts index ff5ba2345..45ac5c3ef 100644 --- a/packages/astro/src/runtime/server/render/component.ts +++ b/packages/astro/src/runtime/server/render/component.ts @@ -5,14 +5,14 @@ import { HTMLBytes, markHTMLString } from '../escape.js'; import { extractDirectives, generateHydrateScript } from '../hydration.js'; import { serializeProps } from '../serialize.js'; import { shorthash } from '../shorthash.js'; -import { renderSlot } from './any.js'; +import { renderSlot, renderSlots } from './slot.js'; import { isAstroComponentFactory, renderAstroComponent, renderTemplate, renderToIterable, } from './astro.js'; -import { Fragment, Renderer } from './common.js'; +import { Fragment, Renderer, stringifyChunk } from './common.js'; import { componentIsHTMLElement, renderHTMLElement } from './dom.js'; import { formatList, internalSpreadAttributes, renderElement, voidElementNames } from './util.js'; @@ -68,18 +68,10 @@ export async function renderComponent( // .html components case 'html': { - const children: Record<string, string> = {}; - if (slots) { - await Promise.all( - Object.entries(slots).map(([key, value]) => - renderSlot(result, value as string).then((output) => { - children[key] = output; - }) - ) - ); - } + const { slotInstructions, children } = await renderSlots(result, slots); const html = (Component as any).render({ slots: children }); - return markHTMLString(html); + const hydrationHtml = slotInstructions ? slotInstructions.map(instr => stringifyChunk(result, instr)).join('') : ''; + return markHTMLString(hydrationHtml + html); } case 'astro-factory': { @@ -130,16 +122,7 @@ Did you mean to add ${formatList(probableRendererNames.map((r) => '`' + r + '`') throw new Error(message); } - const children: Record<string, string> = {}; - if (slots) { - await Promise.all( - Object.entries(slots).map(([key, value]) => - renderSlot(result, value as string).then((output) => { - children[key] = output; - }) - ) - ); - } + const { children, slotInstructions } = await renderSlots(result, slots); // Call the renderers `check` hook to see if any claim this component. let renderer: SSRLoadedRenderer | undefined; @@ -345,6 +328,9 @@ If you're still stuck, please open an issue on GitHub or join us at https://astr } async function* renderAll() { + if(slotInstructions) { + yield * slotInstructions; + } yield { type: 'directive', hydration, result }; yield markHTMLString(renderElement('astro-island', island, false)); } diff --git a/packages/astro/src/runtime/server/render/dom.ts b/packages/astro/src/runtime/server/render/dom.ts index cf6024a88..352b60436 100644 --- a/packages/astro/src/runtime/server/render/dom.ts +++ b/packages/astro/src/runtime/server/render/dom.ts @@ -1,7 +1,7 @@ import type { SSRResult } from '../../../@types/astro'; import { markHTMLString } from '../escape.js'; -import { renderSlot } from './any.js'; +import { renderSlot } from './slot.js'; import { toAttributeString } from './util.js'; export function componentIsHTMLElement(Component: unknown) { diff --git a/packages/astro/src/runtime/server/render/index.ts b/packages/astro/src/runtime/server/render/index.ts index a7de515e2..15bf963cb 100644 --- a/packages/astro/src/runtime/server/render/index.ts +++ b/packages/astro/src/runtime/server/render/index.ts @@ -1,6 +1,6 @@ import { renderTemplate } from './astro.js'; -export { renderSlot } from './any.js'; +export { renderSlot } from './slot.js'; export { renderAstroComponent, renderTemplate, renderToString } from './astro.js'; export { Fragment, Renderer, stringifyChunk } from './common.js'; export { renderComponent } from './component.js'; diff --git a/packages/astro/src/runtime/server/render/slot.ts b/packages/astro/src/runtime/server/render/slot.ts new file mode 100644 index 000000000..a96bc3106 --- /dev/null +++ b/packages/astro/src/runtime/server/render/slot.ts @@ -0,0 +1,59 @@ +import type { RenderInstruction } from './types.js'; +import type { SSRResult } from '../../../@types/astro.js'; + +import { renderChild } from './any.js'; +import { HTMLString, markHTMLString } from '../escape.js'; + +export class SlotString extends HTMLString { + public instructions: null | RenderInstruction[]; + constructor(content: string, instructions: null | RenderInstruction[]) { + super(content); + this.instructions = instructions; + } +} + +export async function renderSlot(_result: any, slotted: string, fallback?: any): Promise<string> { + if (slotted) { + let iterator = renderChild(slotted); + let content = ''; + let instructions: null | RenderInstruction[] = null; + for await (const chunk of iterator) { + if ((chunk as any).type === 'directive') { + if(instructions === null) { + instructions = []; + } + instructions.push(chunk); + } else { + content += chunk; + } + } + return markHTMLString(new SlotString(content, instructions)); + } + return fallback; +} + +interface RenderSlotsResult { + slotInstructions: null | RenderInstruction[], + children: Record<string, string>; +} + +export async function renderSlots(result: SSRResult, slots: any = {}): Promise<RenderSlotsResult> { + let slotInstructions: RenderSlotsResult['slotInstructions'] = null; + let children: RenderSlotsResult['children'] = {}; + if (slots) { + await Promise.all( + Object.entries(slots).map(([key, value]) => + renderSlot(result, value as string).then((output: any) => { + if(output.instructions) { + if(slotInstructions === null) { + slotInstructions = []; + } + slotInstructions.push(...output.instructions); + } + children[key] = output; + }) + ) + ); + } + return { slotInstructions, children }; +} diff --git a/packages/astro/test/astro-slots-nested.test.js b/packages/astro/test/astro-slots-nested.test.js new file mode 100644 index 000000000..58f7153b0 --- /dev/null +++ b/packages/astro/test/astro-slots-nested.test.js @@ -0,0 +1,20 @@ +import { expect } from 'chai'; +import * as cheerio from 'cheerio'; +import { loadFixture } from './test-utils.js'; + +describe('Nested Slots', () => { + let fixture; + + before(async () => { + fixture = await loadFixture({ root: './fixtures/astro-slots-nested/' }); + await fixture.build(); + }); + + it('Hidden nested slots see their hydration scripts hoisted', async () => { + const html = await fixture.readFile('/hidden-nested/index.html'); + const $ = cheerio.load(html); + expect($('script')).to.have.a.lengthOf(1, 'script rendered'); + const scriptInTemplate = $($('template')[0].children[0]).find('script'); + expect(scriptInTemplate).to.have.a.lengthOf(0, 'script defined outside of the inner template'); + }); +}); diff --git a/packages/astro/test/fixtures/astro-slots-nested/astro.config.mjs b/packages/astro/test/fixtures/astro-slots-nested/astro.config.mjs new file mode 100644 index 000000000..9cdfde2d7 --- /dev/null +++ b/packages/astro/test/fixtures/astro-slots-nested/astro.config.mjs @@ -0,0 +1,6 @@ +import { defineConfig } from 'astro/config'; +import react from '@astrojs/react'; + +export default defineConfig({ + integrations: [react()] +}); diff --git a/packages/astro/test/fixtures/astro-slots-nested/package.json b/packages/astro/test/fixtures/astro-slots-nested/package.json new file mode 100644 index 000000000..844b58bea --- /dev/null +++ b/packages/astro/test/fixtures/astro-slots-nested/package.json @@ -0,0 +1,9 @@ +{ + "name": "@test/astro-slots-nested", + "version": "0.0.0", + "private": true, + "dependencies": { + "astro": "workspace:*", + "@astrojs/react": "workspace:*" + } +} diff --git a/packages/astro/test/fixtures/astro-slots-nested/src/components/Inner.tsx b/packages/astro/test/fixtures/astro-slots-nested/src/components/Inner.tsx new file mode 100644 index 000000000..d902a0aef --- /dev/null +++ b/packages/astro/test/fixtures/astro-slots-nested/src/components/Inner.tsx @@ -0,0 +1,3 @@ +export default function Inner() { + return <span>Inner</span>; +} diff --git a/packages/astro/test/fixtures/astro-slots-nested/src/components/Parent.jsx b/packages/astro/test/fixtures/astro-slots-nested/src/components/Parent.jsx new file mode 100644 index 000000000..340dacab3 --- /dev/null +++ b/packages/astro/test/fixtures/astro-slots-nested/src/components/Parent.jsx @@ -0,0 +1,19 @@ +import { useState, useEffect } from 'react'; + +export default function Parent({ children }) { + const [show, setShow] = useState(false); + + useEffect(() => { + console.log('mount'); + }, []); + + return ( + <> + <p> + <input type="checkbox" value={show} onChange={() => setShow(!show)} /> + Toggle show (true should show "Inner") + </p> + {show ? children : 'Nothing'} + </> + ); +} diff --git a/packages/astro/test/fixtures/astro-slots-nested/src/pages/hidden-nested.astro b/packages/astro/test/fixtures/astro-slots-nested/src/pages/hidden-nested.astro new file mode 100644 index 000000000..e51ce00a0 --- /dev/null +++ b/packages/astro/test/fixtures/astro-slots-nested/src/pages/hidden-nested.astro @@ -0,0 +1,18 @@ +--- +import Parent from '../components/Parent' +import Inner from '../components/Inner' +--- + +<html lang="en"> + <head> + <title>Testing</title> + </head> + <body> + <main> + <!-- Try to remove client:load to see it work --> + <Parent client:load> + <Inner client:load /> + </Parent> + </main> + </body> +</html> diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index e12b7f63b..8120e2d64 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1397,6 +1397,14 @@ importers: dependencies: astro: link:../../.. + packages/astro/test/fixtures/astro-slots-nested: + specifiers: + '@astrojs/react': workspace:* + astro: workspace:* + dependencies: + '@astrojs/react': link:../../../../integrations/react + astro: link:../../.. + packages/astro/test/fixtures/before-hydration: specifiers: '@astrojs/preact': workspace:* |