From 1d3a0a16f33aa88c2b60088d6a497e4beaadb2dc Mon Sep 17 00:00:00 2001 From: "Fred K. Schott" Date: Sat, 13 Aug 2022 10:00:12 -0700 Subject: [PATCH] Revert "Ensure hydration scripts inside of slots render ASAP (#4288)" (#4302) * Revert "Ensure hydration scripts inside of slots render ASAP (#4288)" This reverts commit c218100684c90c2b5c490e73b0687ad59d0c58df. * Create khaki-dots-press.md --- .changeset/khaki-dots-press.md | 5 +++ packages/astro/src/@types/astro.ts | 14 -------- packages/astro/src/core/render/result.ts | 1 - .../astro/src/runtime/server/hydration.ts | 8 +++-- packages/astro/src/runtime/server/index.ts | 2 +- packages/astro/src/runtime/server/jsx.ts | 7 ++-- .../astro/src/runtime/server/render/any.ts | 14 +++----- .../astro/src/runtime/server/render/astro.ts | 7 ++-- .../astro/src/runtime/server/render/common.ts | 9 ++--- .../src/runtime/server/render/component.ts | 9 +++-- .../astro/src/runtime/server/render/index.ts | 1 + .../astro/src/runtime/server/render/page.ts | 19 ++--------- .../astro/src/runtime/server/render/types.ts | 8 +++++ .../src/components/Layout.astro | 6 ---- .../hydration-race/src/pages/slot.astro | 15 --------- packages/astro/test/hydration-race.test.js | 33 ------------------- 16 files changed, 44 insertions(+), 114 deletions(-) create mode 100644 .changeset/khaki-dots-press.md create mode 100644 packages/astro/src/runtime/server/render/types.ts delete mode 100644 packages/astro/test/fixtures/hydration-race/src/components/Layout.astro delete mode 100644 packages/astro/test/fixtures/hydration-race/src/pages/slot.astro diff --git a/.changeset/khaki-dots-press.md b/.changeset/khaki-dots-press.md new file mode 100644 index 000000000..fe47b2608 --- /dev/null +++ b/.changeset/khaki-dots-press.md @@ -0,0 +1,5 @@ +--- +"astro": patch +--- + +Revert "Ensure hydration scripts inside of slots render ASAP (#4288)" to fix Svelte integration bug diff --git a/packages/astro/src/@types/astro.ts b/packages/astro/src/@types/astro.ts index 5c8473e58..fdb50889e 100644 --- a/packages/astro/src/@types/astro.ts +++ b/packages/astro/src/@types/astro.ts @@ -1135,25 +1135,11 @@ export interface SSRElement { children: string; } -export interface HydrationMetadata { - directive: string; - value: string; - componentUrl: string; - componentExport: { value: string }; -} - -export interface SSRRenderInstruction { - type: 'directive'; - result: SSRResult; - hydration: HydrationMetadata; -} - export interface SSRMetadata { renderers: SSRLoadedRenderer[]; pathname: string; hasHydrationScript: boolean; hasDirectives: Set; - pendingInstructions: Set; } export interface SSRResult { diff --git a/packages/astro/src/core/render/result.ts b/packages/astro/src/core/render/result.ts index 37b07cb06..32dcdd916 100644 --- a/packages/astro/src/core/render/result.ts +++ b/packages/astro/src/core/render/result.ts @@ -265,7 +265,6 @@ const canonicalURL = new URL(Astro.url.pathname, Astro.site); pathname, hasHydrationScript: false, hasDirectives: new Set(), - pendingInstructions: new Set(), }, response, }; diff --git a/packages/astro/src/runtime/server/hydration.ts b/packages/astro/src/runtime/server/hydration.ts index e57b59d1f..d58dc406e 100644 --- a/packages/astro/src/runtime/server/hydration.ts +++ b/packages/astro/src/runtime/server/hydration.ts @@ -1,6 +1,5 @@ import type { AstroComponentMetadata, - HydrationMetadata, SSRElement, SSRLoadedRenderer, SSRResult, @@ -13,7 +12,12 @@ const HydrationDirectivesRaw = ['load', 'idle', 'media', 'visible', 'only']; const HydrationDirectives = new Set(HydrationDirectivesRaw); export const HydrationDirectiveProps = new Set(HydrationDirectivesRaw.map((n) => `client:${n}`)); -export { HydrationMetadata }; +export interface HydrationMetadata { + directive: string; + value: string; + componentUrl: string; + componentExport: { value: string }; +} interface ExtractedProps { isPage: boolean; diff --git a/packages/astro/src/runtime/server/index.ts b/packages/astro/src/runtime/server/index.ts index c80811843..1304aca3e 100644 --- a/packages/astro/src/runtime/server/index.ts +++ b/packages/astro/src/runtime/server/index.ts @@ -26,7 +26,7 @@ export { stringifyChunk, voidElementNames, } from './render/index.js'; -export type { AstroComponentFactory } from './render/index.js'; +export type { AstroComponentFactory, RenderInstruction } from './render/index.js'; import type { AstroComponentFactory } from './render/index.js'; import { markHTMLString } from './escape.js'; diff --git a/packages/astro/src/runtime/server/jsx.ts b/packages/astro/src/runtime/server/jsx.ts index 665f67348..f7f566788 100644 --- a/packages/astro/src/runtime/server/jsx.ts +++ b/packages/astro/src/runtime/server/jsx.ts @@ -1,15 +1,16 @@ /* eslint-disable no-console */ -import { SSRRenderInstruction, SSRResult } from '../../@types/astro.js'; +import { SSRResult } from '../../@types/astro.js'; import { AstroJSX, isVNode } from '../../jsx-runtime/index.js'; import { escapeHTML, HTMLString, markHTMLString, renderComponent, + RenderInstruction, renderToString, spreadAttributes, stringifyChunk, - voidElementNames, + voidElementNames } from './index.js'; const ClientOnlyPlaceholder = 'astro-client-only'; @@ -121,7 +122,7 @@ export async function renderJSX(result: SSRResult, vnode: any): Promise { } await Promise.all(slotPromises); - let output: string | AsyncIterable; + let output: string | AsyncIterable; if (vnode.type === ClientOnlyPlaceholder && vnode.props['client:only']) { output = await renderComponent( result, diff --git a/packages/astro/src/runtime/server/render/any.ts b/packages/astro/src/runtime/server/render/any.ts index 62d4f5a0c..af55ac099 100644 --- a/packages/astro/src/runtime/server/render/any.ts +++ b/packages/astro/src/runtime/server/render/any.ts @@ -1,7 +1,7 @@ -import { SSRRenderInstruction, SSRResult } from '../../../@types/astro'; import { escapeHTML, HTMLString, markHTMLString } from '../escape.js'; import { AstroComponent, renderAstroComponent } from './astro.js'; - +import { stringifyChunk } from './common.js'; + export async function* renderChild(child: any): AsyncIterable { child = await child; if (child instanceof HTMLString) { @@ -34,19 +34,13 @@ export async function* renderChild(child: any): AsyncIterable { } } -export async function renderSlot( - result: SSRResult, - slotted: string, - fallback?: any -): Promise { +export async function renderSlot(result: any, slotted: string, fallback?: any): Promise { if (slotted) { let iterator = renderChild(slotted); let content = ''; for await (const chunk of iterator) { if ((chunk as any).type === 'directive') { - // Do not render directives scripts in a slot, instead mark them as pending - // so that renderPage includes them ASAP. - result._metadata.pendingInstructions.add(chunk as SSRRenderInstruction); + content += stringifyChunk(result, chunk); } else { content += chunk; } diff --git a/packages/astro/src/runtime/server/render/astro.ts b/packages/astro/src/runtime/server/render/astro.ts index f328ce973..b7e47a68c 100644 --- a/packages/astro/src/runtime/server/render/astro.ts +++ b/packages/astro/src/runtime/server/render/astro.ts @@ -1,5 +1,6 @@ -import type { SSRRenderInstruction, SSRResult } from '../../../@types/astro'; +import type { SSRResult } from '../../../@types/astro'; import type { AstroComponentFactory } from './index'; +import type { RenderInstruction } from './types'; import { markHTMLString } from '../escape.js'; import { HydrationDirectiveProps } from '../hydration.js'; @@ -57,7 +58,7 @@ export function isAstroComponent(obj: any): obj is AstroComponent { export async function* renderAstroComponent( component: InstanceType -): AsyncIterable { +): AsyncIterable { for await (const value of component) { if (value || value === 0) { for await (const chunk of renderChild(value)) { @@ -103,7 +104,7 @@ export async function renderToIterable( displayName: string, props: any, children: any -): Promise> { +): Promise> { validateComponentProps(props, displayName); const Component = await componentFactory(result, props, children); diff --git a/packages/astro/src/runtime/server/render/common.ts b/packages/astro/src/runtime/server/render/common.ts index 61a2377cc..cbb342e68 100644 --- a/packages/astro/src/runtime/server/render/common.ts +++ b/packages/astro/src/runtime/server/render/common.ts @@ -1,11 +1,12 @@ -import type { SSRRenderInstruction, SSRResult } from '../../../@types/astro'; +import type { SSRResult } from '../../../@types/astro'; +import type { RenderInstruction } from './types.js'; import { markHTMLString } from '../escape.js'; import { determineIfNeedsHydrationScript, determinesIfNeedsDirectiveScript, getPrescripts, - PrescriptType, + PrescriptType } from '../scripts.js'; export const Fragment = Symbol.for('astro:fragment'); @@ -14,10 +15,10 @@ export const Renderer = Symbol.for('astro:renderer'); // Rendering produces either marked strings of HTML or instructions for hydration. // These directive instructions bubble all the way up to renderPage so that we // can ensure they are added only once, and as soon as possible. -export function stringifyChunk(result: SSRResult, chunk: string | SSRRenderInstruction) { +export function stringifyChunk(result: SSRResult, chunk: string | RenderInstruction) { switch ((chunk as any).type) { case 'directive': { - const { hydration } = chunk as SSRRenderInstruction; + const { hydration } = chunk as RenderInstruction; let needsHydrationScript = hydration && determineIfNeedsHydrationScript(result); let needsDirectiveScript = hydration && determinesIfNeedsDirectiveScript(result, hydration.directive); diff --git a/packages/astro/src/runtime/server/render/component.ts b/packages/astro/src/runtime/server/render/component.ts index 34740ab24..ea1c3a9ed 100644 --- a/packages/astro/src/runtime/server/render/component.ts +++ b/packages/astro/src/runtime/server/render/component.ts @@ -1,9 +1,8 @@ import type { AstroComponentMetadata, - SSRLoadedRenderer, - SSRRenderInstruction, - SSRResult, + SSRLoadedRenderer, SSRResult } from '../../../@types/astro'; +import type { RenderInstruction } from './types.js'; import { markHTMLString } from '../escape.js'; import { extractDirectives, generateHydrateScript } from '../hydration.js'; @@ -53,7 +52,7 @@ export async function renderComponent( Component: unknown, _props: Record, slots: any = {} -): Promise> { +): Promise> { Component = await Component; switch (getComponentType(Component)) { @@ -83,7 +82,7 @@ export async function renderComponent( case 'astro-factory': { async function* renderAstroComponentInline(): AsyncGenerator< - string | SSRRenderInstruction, + string | RenderInstruction, void, undefined > { diff --git a/packages/astro/src/runtime/server/render/index.ts b/packages/astro/src/runtime/server/render/index.ts index 1567307d5..a7de515e2 100644 --- a/packages/astro/src/runtime/server/render/index.ts +++ b/packages/astro/src/runtime/server/render/index.ts @@ -7,6 +7,7 @@ export { renderComponent } from './component.js'; export { renderHTMLElement } from './dom.js'; export { maybeRenderHead, renderHead } from './head.js'; export { renderPage } from './page.js'; +export type { RenderInstruction } from './types'; export { addAttribute, defineScriptVars, voidElementNames } from './util.js'; // The callback passed to to $$createComponent diff --git a/packages/astro/src/runtime/server/render/page.ts b/packages/astro/src/runtime/server/render/page.ts index 85dbec01e..9e2c0fd2b 100644 --- a/packages/astro/src/runtime/server/render/page.ts +++ b/packages/astro/src/runtime/server/render/page.ts @@ -49,28 +49,13 @@ export async function renderPage( let headers = new Headers(init.headers); let body: BodyInit; - // Combines HTML chunks coming from the iterable with rendering instructions - // added to metadata. These instructions need to go out first to ensure - // the scripts exist before the islands that need them. - async function* eachChunk() { - for await (const chunk of iterable) { - if (result._metadata.pendingInstructions.size) { - for (const instr of result._metadata.pendingInstructions) { - result._metadata.pendingInstructions.delete(instr); - yield instr; - } - } - yield chunk; - } - } - if (streaming) { body = new ReadableStream({ start(controller) { async function read() { let i = 0; try { - for await (const chunk of eachChunk()) { + for await (const chunk of iterable) { let html = stringifyChunk(result, chunk); if (i === 0) { @@ -92,7 +77,7 @@ export async function renderPage( } else { body = ''; let i = 0; - for await (const chunk of eachChunk()) { + for await (const chunk of iterable) { let html = stringifyChunk(result, chunk); if (i === 0) { if (!/ - diff --git a/packages/astro/test/fixtures/hydration-race/src/pages/slot.astro b/packages/astro/test/fixtures/hydration-race/src/pages/slot.astro deleted file mode 100644 index 7f054d45f..000000000 --- a/packages/astro/test/fixtures/hydration-race/src/pages/slot.astro +++ /dev/null @@ -1,15 +0,0 @@ ---- -import Layout from '../components/Layout.astro'; -import Wrapper from '../components/Wrapper.astro'; -import One from '../components/One.jsx'; ---- - - - Testing - - - - - - - diff --git a/packages/astro/test/hydration-race.test.js b/packages/astro/test/hydration-race.test.js index 803a65a00..d35681557 100644 --- a/packages/astro/test/hydration-race.test.js +++ b/packages/astro/test/hydration-race.test.js @@ -26,37 +26,4 @@ describe('Hydration script ordering', async () => { expect($('style')).to.have.a.lengthOf(1, 'hydration style added once'); expect($('script')).to.have.a.lengthOf(1, 'only one hydration script needed'); }); - - it('Hydration placed in correct location when there is slots used', async () => { - let html = await fixture.readFile('/slot/index.html'); - let $ = cheerio.load(html); - //console.log(html); - - // First, let's make sure all islands rendered (or test is bad) - expect($('astro-island')).to.have.a.lengthOf(3); - - // Now let's make sure the hydration script is placed before the first component - let firstIsland = $($('astro-island').get(0)); - let el = firstIsland.prev(); - let foundScript = false; - - // Traverse the DOM going backwards until we find a script, if there is one. - while (el.length) { - let last = el; - while (el.length) { - if (el.prop('tagName') === 'SCRIPT') { - foundScript = true; - } - last = el; - el = el.prev(); - } - el = last.parent(); - } - - expect(foundScript).to.be.true; - - // Sanity check that we're only rendering them once. - expect($('style')).to.have.a.lengthOf(1, 'hydration style added once'); - expect($('script')).to.have.a.lengthOf(1, 'only one hydration script needed'); - }); });