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

Pass children to client components even if they do not render them #2588

Merged
merged 5 commits into from
Feb 16, 2022
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
5 changes: 5 additions & 0 deletions .changeset/great-suns-pump.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Fix for passing children to client component when the component does not render them
1 change: 1 addition & 0 deletions packages/astro/src/@types/astro.ts
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,7 @@ export type GetStaticPathsResultKeyed = GetStaticPathsResult & {
};

export interface HydrateOptions {
name: string;
value?: string;
}

Expand Down
20 changes: 18 additions & 2 deletions packages/astro/src/runtime/client/idle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,26 @@ import type { GetHydrateCallback, HydrateOptions } from '../../@types/astro';
* Hydrate this component as soon as the main thread is free
* (or after a short delay, if `requestIdleCallback`) isn't supported
*/
export default async function onIdle(astroId: string, _options: HydrateOptions, getHydrateCallback: GetHydrateCallback) {
export default async function onIdle(astroId: string, options: HydrateOptions, getHydrateCallback: GetHydrateCallback) {
const cb = async () => {
const roots = document.querySelectorAll(`astro-root[uid="${astroId}"]`);
const innerHTML = roots[0].querySelector(`astro-fragment`)?.innerHTML ?? null;
if(roots.length === 0) {
throw new Error(`Unable to find the root for the component ${options.name}`);
}

let innerHTML: string | null = null;
let fragment = roots[0].querySelector(`astro-fragment`);
if(fragment == null && roots[0].hasAttribute('tmpl')) {
// If there is no child fragment, check to see if there is a template.
// This happens if children were passed but the client component did not render any.
let template = roots[0].querySelector(`template[data-astro-template]`);
if(template) {
innerHTML = template.innerHTML;
template.remove();
}
} else if(fragment) {
innerHTML = fragment.innerHTML;
}
const hydrate = await getHydrateCallback();

for (const root of roots) {
Expand Down
22 changes: 20 additions & 2 deletions packages/astro/src/runtime/client/load.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,27 @@ import type { GetHydrateCallback, HydrateOptions } from '../../@types/astro';
/**
* Hydrate this component immediately
*/
export default async function onLoad(astroId: string, _options: HydrateOptions, getHydrateCallback: GetHydrateCallback) {
export default async function onLoad(astroId: string, options: HydrateOptions, getHydrateCallback: GetHydrateCallback) {
const roots = document.querySelectorAll(`astro-root[uid="${astroId}"]`);
const innerHTML = roots[0].querySelector(`astro-fragment`)?.innerHTML ?? null;
if(roots.length === 0) {
throw new Error(`Unable to find the root for the component ${options.name}`);
}

let innerHTML: string | null = null;
let fragment = roots[0].querySelector(`astro-fragment`);
if(fragment == null && roots[0].hasAttribute('tmpl')) {
// If there is no child fragment, check to see if there is a template.
// This happens if children were passed but the client component did not render any.
let template = roots[0].querySelector(`template[data-astro-template]`);
if(template) {
innerHTML = template.innerHTML;
template.remove();
}
} else if(fragment) {
innerHTML = fragment.innerHTML;
}

//const innerHTML = roots[0].querySelector(`astro-fragment`)?.innerHTML ?? null;
const hydrate = await getHydrateCallback();

for (const root of roots) {
Expand Down
18 changes: 17 additions & 1 deletion packages/astro/src/runtime/client/media.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,23 @@ import type { GetHydrateCallback, HydrateOptions } from '../../@types/astro';
*/
export default async function onMedia(astroId: string, options: HydrateOptions, getHydrateCallback: GetHydrateCallback) {
const roots = document.querySelectorAll(`astro-root[uid="${astroId}"]`);
const innerHTML = roots[0].querySelector(`astro-fragment`)?.innerHTML ?? null;
if(roots.length === 0) {
throw new Error(`Unable to find the root for the component ${options.name}`);
}

let innerHTML: string | null = null;
let fragment = roots[0].querySelector(`astro-fragment`);
if(fragment == null && roots[0].hasAttribute('tmpl')) {
// If there is no child fragment, check to see if there is a template.
// This happens if children were passed but the client component did not render any.
let template = roots[0].querySelector(`template[data-astro-template]`);
if(template) {
innerHTML = template.innerHTML;
template.remove();
}
} else if(fragment) {
innerHTML = fragment.innerHTML;
}

const cb = async () => {
const hydrate = await getHydrateCallback();
Expand Down
20 changes: 18 additions & 2 deletions packages/astro/src/runtime/client/only.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,25 @@ import type { GetHydrateCallback, HydrateOptions } from '../../@types/astro';
/**
* Hydrate this component immediately
*/
export default async function onLoad(astroId: string, _options: HydrateOptions, getHydrateCallback: GetHydrateCallback) {
export default async function onLoad(astroId: string, options: HydrateOptions, getHydrateCallback: GetHydrateCallback) {
const roots = document.querySelectorAll(`astro-root[uid="${astroId}"]`);
const innerHTML = roots[0].querySelector(`astro-fragment`)?.innerHTML ?? null;
if(roots.length === 0) {
throw new Error(`Unable to find the root for the component ${options.name}`);
}

let innerHTML: string | null = null;
let fragment = roots[0].querySelector(`astro-fragment`);
if(fragment == null && roots[0].hasAttribute('tmpl')) {
// If there is no child fragment, check to see if there is a template.
// This happens if children were passed but the client component did not render any.
let template = roots[0].querySelector(`template[data-astro-template]`);
if(template) {
innerHTML = template.innerHTML;
template.remove();
}
} else if(fragment) {
innerHTML = fragment.innerHTML;
}
const hydrate = await getHydrateCallback();

for (const root of roots) {
Expand Down
20 changes: 18 additions & 2 deletions packages/astro/src/runtime/client/visible.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,25 @@ import type { GetHydrateCallback, HydrateOptions } from '../../@types/astro';
* We target the children because `astro-root` is set to `display: contents`
* which doesn't work with IntersectionObserver
*/
export default async function onVisible(astroId: string, _options: HydrateOptions, getHydrateCallback: GetHydrateCallback) {
export default async function onVisible(astroId: string, options: HydrateOptions, getHydrateCallback: GetHydrateCallback) {
const roots = document.querySelectorAll(`astro-root[uid="${astroId}"]`);
const innerHTML = roots[0].querySelector(`astro-fragment`)?.innerHTML ?? null;
if(roots.length === 0) {
throw new Error(`Unable to find the root for the component ${options.name}`);
}

let innerHTML: string | null = null;
let fragment = roots[0].querySelector(`astro-fragment`);
if(fragment == null && roots[0].hasAttribute('tmpl')) {
// If there is no child fragment, check to see if there is a template.
// This happens if children were passed but the client component did not render any.
let template = roots[0].querySelector(`template[data-astro-template]`);
if(template) {
innerHTML = template.innerHTML;
template.remove();
}
} else if(fragment) {
innerHTML = fragment.innerHTML;
}

const cb = async () => {
const hydrate = await getHydrateCallback();
Expand Down
2 changes: 1 addition & 1 deletion packages/astro/src/runtime/server/hydration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ export async function generateHydrateScript(scriptOptions: HydrateScriptOptions,
const hydrationScript = {
props: { type: 'module', 'data-astro-component-hydration': true },
children: `import setup from '${await result.resolve(hydrationSpecifier(hydrate))}';
setup("${astroId}", {${metadata.hydrateArgs ? `value: ${JSON.stringify(metadata.hydrateArgs)}` : ''}}, async () => {
setup("${astroId}", {name:"${metadata.displayName}",${metadata.hydrateArgs ? `value: ${JSON.stringify(metadata.hydrateArgs)}` : ''}}, async () => {
${hydrationSource}
});
`,
Expand Down
5 changes: 4 additions & 1 deletion packages/astro/src/runtime/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,10 @@ If you're still stuck, please open an issue on GitHub or join us at https://astr
// INVESTIGATE: This will likely be a problem in streaming because the `<head>` will be gone at this point.
result.scripts.add(await generateHydrateScript({ renderer, result, astroId, props }, metadata as Required<AstroComponentMetadata>));

return unescapeHTML(`<astro-root uid="${astroId}">${html ?? ''}</astro-root>`);
// Render a template if no fragment is provided.
const needsAstroTemplate = children && !/<\/?astro-fragment\>/.test(html);
const template = needsAstroTemplate ? `<template data-astro-template>${children}</template>` : '';
return unescapeHTML(`<astro-root uid="${astroId}"${needsAstroTemplate ? ' tmpl' : ''}>${html ?? ''}${template}</astro-root>`);
}

/** Create the Astro.fetchContent() runtime function. */
Expand Down
20 changes: 20 additions & 0 deletions packages/astro/test/astro-children.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,4 +69,24 @@ describe('Component children', () => {
expect($svelte.children(':first-child').text().trim()).to.equal('Hello world');
expect($svelte.children(':last-child').text().trim()).to.equal('Goodbye world');
});

it('Renders a template when children are not rendered for client components', async () => {
const html = await fixture.readFile('/no-render/index.html');
const $ = cheerio.load(html);

// test 1: If SSR only, no children are rendered.
expect($('#ssr-only').children()).to.have.lengthOf(0);

// test 2: If client, and no children are rendered, a template is.
expect($('#client').parent().children()).to.have.lengthOf(2, 'rendered the client component and a template');
expect($('#client').parent().find('template[data-astro-template]')).to.have.lengthOf(1, 'Found 1 template');

// test 3: If client, and children are rendered, no template is.
expect($('#client-render').parent().children()).to.have.lengthOf(1);
expect($('#client-render').parent().find('template')).to.have.lengthOf(0);

// test 4: If client and no children are provided, no template is.
expect($('#client-no-children').parent().children()).to.have.lengthOf(1);
expect($('#client-no-children').parent().find('template')).to.have.lengthOf(0);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { h } from 'preact';

export default function PreactComponent({ id, children, render = false }) {
return <div id={id} class="preact-no-children">{render && children}</div>;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
---
import PreactComponent from '../components/NoRender.jsx';
---
<html>
<head><title>Children</title></head>
<body>
<PreactComponent id="ssr-only">
<h1>Hello world</h1>
<h1>Goodbye world</h1>
</PreactComponent>
<PreactComponent id="client" client:load>
<h1>Hello world</h1>
<h1>Goodbye world</h1>
</PreactComponent>
<PreactComponent id="client-render" render={true} client:load>
<h1>Hello world</h1>
<h1>Goodbye world</h1>
</PreactComponent>

<PreactComponent id="client-no-children" client:load></PreactComponent>
</body>
</html>