Skip to content

Commit

Permalink
Revert "Revert "Ensure hydration scripts inside of slots render ASAP (#…
Browse files Browse the repository at this point in the history
…4288)" (#4302)"

This reverts commit 1d3a0a1.
  • Loading branch information
matthewp committed Aug 15, 2022
1 parent 63758ae commit c2cfe9b
Show file tree
Hide file tree
Showing 16 changed files with 122 additions and 36 deletions.
14 changes: 14 additions & 0 deletions packages/astro/src/@types/astro.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1135,11 +1135,25 @@ 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<string>;
pendingInstructions: Set<SSRRenderInstruction>;
}

export interface SSRResult {
Expand Down
1 change: 1 addition & 0 deletions packages/astro/src/core/render/result.ts
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,7 @@ const canonicalURL = new URL(Astro.url.pathname, Astro.site);
pathname,
hasHydrationScript: false,
hasDirectives: new Set(),
pendingInstructions: new Set(),
},
response,
};
Expand Down
8 changes: 2 additions & 6 deletions packages/astro/src/runtime/server/hydration.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type {
AstroComponentMetadata,
HydrationMetadata,
SSRElement,
SSRLoadedRenderer,
SSRResult,
Expand All @@ -12,12 +13,7 @@ const HydrationDirectivesRaw = ['load', 'idle', 'media', 'visible', 'only'];
const HydrationDirectives = new Set(HydrationDirectivesRaw);
export const HydrationDirectiveProps = new Set(HydrationDirectivesRaw.map((n) => `client:${n}`));

export interface HydrationMetadata {
directive: string;
value: string;
componentUrl: string;
componentExport: { value: string };
}
export { HydrationMetadata };

interface ExtractedProps {
isPage: boolean;
Expand Down
2 changes: 1 addition & 1 deletion packages/astro/src/runtime/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export {
stringifyChunk,
voidElementNames,
} from './render/index.js';
export type { AstroComponentFactory, RenderInstruction } from './render/index.js';
export type { AstroComponentFactory } from './render/index.js';
import type { AstroComponentFactory } from './render/index.js';

import { markHTMLString } from './escape.js';
Expand Down
5 changes: 2 additions & 3 deletions packages/astro/src/runtime/server/jsx.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
/* eslint-disable no-console */
import { SSRResult } from '../../@types/astro.js';
import { SSRRenderInstruction, SSRResult } from '../../@types/astro.js';
import { AstroJSX, isVNode } from '../../jsx-runtime/index.js';
import {
escapeHTML,
HTMLString,
markHTMLString,
renderComponent,
RenderInstruction,
renderToString,
spreadAttributes,
stringifyChunk,
Expand Down Expand Up @@ -122,7 +121,7 @@ export async function renderJSX(result: SSRResult, vnode: any): Promise<any> {
}
await Promise.all(slotPromises);

let output: string | AsyncIterable<string | RenderInstruction>;
let output: string | AsyncIterable<string | SSRRenderInstruction>;
if (vnode.type === ClientOnlyPlaceholder && vnode.props['client:only']) {
output = await renderComponent(
result,
Expand Down
12 changes: 9 additions & 3 deletions packages/astro/src/runtime/server/render/any.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
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<any> {
child = await child;
Expand Down Expand Up @@ -34,13 +34,19 @@ export async function* renderChild(child: any): AsyncIterable<any> {
}
}

export async function renderSlot(result: any, slotted: string, fallback?: any): Promise<string> {
export async function renderSlot(
result: SSRResult,
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);
// 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);
} else {
content += chunk;
}
Expand Down
7 changes: 3 additions & 4 deletions packages/astro/src/runtime/server/render/astro.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import type { SSRResult } from '../../../@types/astro';
import type { SSRRenderInstruction, SSRResult } from '../../../@types/astro';
import type { AstroComponentFactory } from './index';
import type { RenderInstruction } from './types';

import { markHTMLString } from '../escape.js';
import { HydrationDirectiveProps } from '../hydration.js';
Expand Down Expand Up @@ -58,7 +57,7 @@ export function isAstroComponent(obj: any): obj is AstroComponent {

export async function* renderAstroComponent(
component: InstanceType<typeof AstroComponent>
): AsyncIterable<string | RenderInstruction> {
): AsyncIterable<string | SSRRenderInstruction> {
for await (const value of component) {
if (value || value === 0) {
for await (const chunk of renderChild(value)) {
Expand Down Expand Up @@ -104,7 +103,7 @@ export async function renderToIterable(
displayName: string,
props: any,
children: any
): Promise<AsyncIterable<string | RenderInstruction>> {
): Promise<AsyncIterable<string | SSRRenderInstruction>> {
validateComponentProps(props, displayName);
const Component = await componentFactory(result, props, children);

Expand Down
7 changes: 3 additions & 4 deletions packages/astro/src/runtime/server/render/common.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import type { SSRResult } from '../../../@types/astro';
import type { RenderInstruction } from './types.js';
import type { SSRRenderInstruction, SSRResult } from '../../../@types/astro';

import { markHTMLString } from '../escape.js';
import {
Expand All @@ -15,10 +14,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 | RenderInstruction) {
export function stringifyChunk(result: SSRResult, chunk: string | SSRRenderInstruction) {
switch ((chunk as any).type) {
case 'directive': {
const { hydration } = chunk as RenderInstruction;
const { hydration } = chunk as SSRRenderInstruction;
let needsHydrationScript = hydration && determineIfNeedsHydrationScript(result);
let needsDirectiveScript =
hydration && determinesIfNeedsDirectiveScript(result, hydration.directive);
Expand Down
12 changes: 8 additions & 4 deletions packages/astro/src/runtime/server/render/component.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
import type { AstroComponentMetadata, SSRLoadedRenderer, SSRResult } from '../../../@types/astro';
import type { RenderInstruction } from './types.js';
import type {
AstroComponentMetadata,
SSRLoadedRenderer,
SSRRenderInstruction,
SSRResult,
} from '../../../@types/astro';

import { markHTMLString } from '../escape.js';
import { extractDirectives, generateHydrateScript } from '../hydration.js';
Expand Down Expand Up @@ -49,7 +53,7 @@ export async function renderComponent(
Component: unknown,
_props: Record<string | number, any>,
slots: any = {}
): Promise<string | AsyncIterable<string | RenderInstruction>> {
): Promise<string | AsyncIterable<string | SSRRenderInstruction>> {
Component = await Component;

switch (getComponentType(Component)) {
Expand Down Expand Up @@ -79,7 +83,7 @@ export async function renderComponent(

case 'astro-factory': {
async function* renderAstroComponentInline(): AsyncGenerator<
string | RenderInstruction,
string | SSRRenderInstruction,
void,
undefined
> {
Expand Down
1 change: 0 additions & 1 deletion packages/astro/src/runtime/server/render/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ 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
Expand Down
19 changes: 17 additions & 2 deletions packages/astro/src/runtime/server/render/page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,28 @@ 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 iterable) {
for await (const chunk of eachChunk()) {
let html = stringifyChunk(result, chunk);

if (i === 0) {
Expand All @@ -77,7 +92,7 @@ export async function renderPage(
} else {
body = '';
let i = 0;
for await (const chunk of iterable) {
for await (const chunk of eachChunk()) {
let html = stringifyChunk(result, chunk);
if (i === 0) {
if (!/<!doctype html/i.test(html)) {
Expand Down
8 changes: 0 additions & 8 deletions packages/astro/src/runtime/server/render/types.ts

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
import Wrapper from './Wrapper.astro';
---

<Wrapper />
<slot />
15 changes: 15 additions & 0 deletions packages/astro/test/fixtures/hydration-race/src/pages/slot.astro
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
---
import Layout from '../components/Layout.astro';
import Wrapper from '../components/Wrapper.astro';
import One from '../components/One.jsx';
---
<html>
<head>
<title>Testing</title>
</head>
<body>
<Layout>
<One name="Three" client:idle />
</Layout>
</body>
</html>
33 changes: 33 additions & 0 deletions packages/astro/test/hydration-race.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,37 @@ 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');
});
});
8 changes: 8 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit c2cfe9b

Please sign in to comment.