From 441e55701f6551ab8d3f26f08e1e8dd9ace13f3b Mon Sep 17 00:00:00 2001 From: Nate Moore Date: Thu, 23 Jun 2022 10:10:54 -0500 Subject: [PATCH] Enable named slots in renderers (#3652) * feat: pass all slots to renderers * refactor: pass `slots` as top-level props * test: add named slot test for frameworks * fix: nested hydration, slots that are not initially rendered * test: add nested-recursive e2e test * fix: render unmatched custom element children * chore: update lockfile * fix: unrendered slots for client:only * fix(lit): ensure lit integration uses new slots API * chore: add changeset * chore: add changesets * fix: lit slots * feat: convert dash-case or snake_case slots to camelCase for JSX * feat: remove tmpl special logic * test: add slot components-in-markdown test * refactor: prefer Object.entries.map() to for/of loop Co-authored-by: Nate Moore --- .../nested-recursive/astro.config.mjs | 12 +++ e2e/fixtures/nested-recursive/package.json | 24 +++++ .../src/components/PreactCounter.tsx | 17 ++++ .../src/components/ReactCounter.jsx | 17 ++++ .../src/components/SolidCounter.tsx | 17 ++++ .../src/components/SvelteCounter.svelte | 29 ++++++ .../src/components/VueCounter.vue | 34 +++++++ .../nested-recursive/src/pages/index.astro | 28 ++++++ e2e/nested-recursive.test.js | 96 +++++++++++++++++++ src/@types/astro.ts | 2 +- src/runtime/server/astro-island.ts | 27 +++--- src/runtime/server/index.ts | 51 +++++++--- .../lit-element/src/components/my-element.js | 4 + .../lit-element/src/pages/slots.astro | 15 +++ .../slots-preact/src/components/Counter.jsx | 4 +- .../slots-preact/src/pages/index.astro | 2 + .../slots-preact/src/pages/markdown.md | 9 ++ .../slots-react/src/components/Counter.jsx | 4 +- .../slots-react/src/pages/index.astro | 2 + .../slots-react/src/pages/markdown.md | 9 ++ .../slots-solid/src/components/Counter.jsx | 4 +- .../slots-solid/src/pages/index.astro | 2 + .../slots-solid/src/pages/markdown.md | 9 ++ .../src/components/Counter.svelte | 4 +- .../slots-svelte/src/pages/index.astro | 8 +- .../slots-svelte/src/pages/markdown.md | 9 ++ .../slots-vue/src/components/Counter.vue | 2 + test/fixtures/slots-vue/src/pages/index.astro | 2 + test/fixtures/slots-vue/src/pages/markdown.md | 9 ++ test/lit-element.test.js | 19 ++++ test/slots-preact.test.js | 32 +++++++ test/slots-react.test.js | 32 +++++++ test/slots-solid.test.js | 32 +++++++ test/slots-svelte.test.js | 34 ++++++- test/slots-vue.test.js | 32 +++++++ 35 files changed, 597 insertions(+), 36 deletions(-) create mode 100644 e2e/fixtures/nested-recursive/astro.config.mjs create mode 100644 e2e/fixtures/nested-recursive/package.json create mode 100644 e2e/fixtures/nested-recursive/src/components/PreactCounter.tsx create mode 100644 e2e/fixtures/nested-recursive/src/components/ReactCounter.jsx create mode 100644 e2e/fixtures/nested-recursive/src/components/SolidCounter.tsx create mode 100644 e2e/fixtures/nested-recursive/src/components/SvelteCounter.svelte create mode 100644 e2e/fixtures/nested-recursive/src/components/VueCounter.vue create mode 100644 e2e/fixtures/nested-recursive/src/pages/index.astro create mode 100644 e2e/nested-recursive.test.js create mode 100644 test/fixtures/lit-element/src/pages/slots.astro create mode 100644 test/fixtures/slots-preact/src/pages/markdown.md create mode 100644 test/fixtures/slots-react/src/pages/markdown.md create mode 100644 test/fixtures/slots-solid/src/pages/markdown.md create mode 100644 test/fixtures/slots-svelte/src/pages/markdown.md create mode 100644 test/fixtures/slots-vue/src/pages/markdown.md diff --git a/e2e/fixtures/nested-recursive/astro.config.mjs b/e2e/fixtures/nested-recursive/astro.config.mjs new file mode 100644 index 000000000000..4b50887cd70c --- /dev/null +++ b/e2e/fixtures/nested-recursive/astro.config.mjs @@ -0,0 +1,12 @@ +import { defineConfig } from 'astro/config'; +import preact from '@astrojs/preact'; +import react from '@astrojs/react'; +import svelte from '@astrojs/svelte'; +import vue from '@astrojs/vue'; +import solid from '@astrojs/solid-js'; + +// https://astro.build/config +export default defineConfig({ + // Enable many frameworks to support all different kinds of components. + integrations: [preact(), react(), svelte(), vue(), solid()], +}); diff --git a/e2e/fixtures/nested-recursive/package.json b/e2e/fixtures/nested-recursive/package.json new file mode 100644 index 000000000000..3376ef59616f --- /dev/null +++ b/e2e/fixtures/nested-recursive/package.json @@ -0,0 +1,24 @@ +{ + "name": "@e2e/nested-recursive", + "version": "0.0.0", + "private": true, + "devDependencies": { + "@astrojs/preact": "workspace:*", + "@astrojs/react": "workspace:*", + "@astrojs/solid-js": "workspace:*", + "@astrojs/svelte": "workspace:*", + "@astrojs/vue": "workspace:*", + "astro": "workspace:*" + }, + "dependencies": { + "preact": "^10.7.3", + "react": "^18.1.0", + "react-dom": "^18.1.0", + "solid-js": "^1.4.3", + "svelte": "^3.48.0", + "vue": "^3.2.36" + }, + "scripts": { + "dev": "astro dev" + } +} diff --git a/e2e/fixtures/nested-recursive/src/components/PreactCounter.tsx b/e2e/fixtures/nested-recursive/src/components/PreactCounter.tsx new file mode 100644 index 000000000000..32200f41f64b --- /dev/null +++ b/e2e/fixtures/nested-recursive/src/components/PreactCounter.tsx @@ -0,0 +1,17 @@ +import { useState } from 'preact/hooks'; + +/** a counter written in Preact */ +export default function PreactCounter({ children, id }) { + const [count, setCount] = useState(0); + const add = () => setCount((i) => i + 1); + const subtract = () => setCount((i) => i - 1); + + return ( +
+ + 
{count}
+ +
{children}
+
+ ); +} diff --git a/e2e/fixtures/nested-recursive/src/components/ReactCounter.jsx b/e2e/fixtures/nested-recursive/src/components/ReactCounter.jsx new file mode 100644 index 000000000000..6b3a1de5f167 --- /dev/null +++ b/e2e/fixtures/nested-recursive/src/components/ReactCounter.jsx @@ -0,0 +1,17 @@ +import { useState } from 'react'; + +/** a counter written in React */ +export default function ReactCounter({ children, id }) { + const [count, setCount] = useState(0); + const add = () => setCount((i) => i + 1); + const subtract = () => setCount((i) => i - 1); + + return ( +
+ + 
{count}
+ +
{children}
+
+ ); +} diff --git a/e2e/fixtures/nested-recursive/src/components/SolidCounter.tsx b/e2e/fixtures/nested-recursive/src/components/SolidCounter.tsx new file mode 100644 index 000000000000..afabe43b9e81 --- /dev/null +++ b/e2e/fixtures/nested-recursive/src/components/SolidCounter.tsx @@ -0,0 +1,17 @@ +import { createSignal } from 'solid-js'; + +/** a counter written with Solid */ +export default function SolidCounter({ children, id }) { + const [count, setCount] = createSignal(0); + const add = () => setCount(count() + 1); + const subtract = () => setCount(count() - 1); + + return ( +
+ + 
{count()}
+ +
{children}
+
+ ); +} diff --git a/e2e/fixtures/nested-recursive/src/components/SvelteCounter.svelte b/e2e/fixtures/nested-recursive/src/components/SvelteCounter.svelte new file mode 100644 index 000000000000..733f58076a24 --- /dev/null +++ b/e2e/fixtures/nested-recursive/src/components/SvelteCounter.svelte @@ -0,0 +1,29 @@ + + + +
+ + 
{ count }
+ +
+ +
+
+ + diff --git a/e2e/fixtures/nested-recursive/src/components/VueCounter.vue b/e2e/fixtures/nested-recursive/src/components/VueCounter.vue new file mode 100644 index 000000000000..d404cc965c52 --- /dev/null +++ b/e2e/fixtures/nested-recursive/src/components/VueCounter.vue @@ -0,0 +1,34 @@ + + + diff --git a/e2e/fixtures/nested-recursive/src/pages/index.astro b/e2e/fixtures/nested-recursive/src/pages/index.astro new file mode 100644 index 000000000000..685c7fb5ed70 --- /dev/null +++ b/e2e/fixtures/nested-recursive/src/pages/index.astro @@ -0,0 +1,28 @@ +--- +import ReactCounter from '../components/ReactCounter.jsx'; +import PreactCounter from '../components/PreactCounter.tsx'; +import SolidCounter from '../components/SolidCounter.tsx'; +import VueCounter from '../components/VueCounter.vue'; +import SvelteCounter from '../components/SvelteCounter.svelte'; +--- + + + + + + + + +
+ + + + + + + + + +
+ + diff --git a/e2e/nested-recursive.test.js b/e2e/nested-recursive.test.js new file mode 100644 index 000000000000..ae981189a619 --- /dev/null +++ b/e2e/nested-recursive.test.js @@ -0,0 +1,96 @@ +import { test as base, expect } from '@playwright/test'; +import { loadFixture } from './test-utils.js'; + +const test = base.extend({ + astro: async ({}, use) => { + const fixture = await loadFixture({ root: './fixtures/nested-recursive/' }); + await use(fixture); + }, +}); + +let devServer; + +test.beforeEach(async ({ astro }) => { + devServer = await astro.startDevServer(); +}); + +test.afterEach(async () => { + await devServer.stop(); +}); + +test.describe('Recursive Nested Frameworks', () => { + test('React counter', async ({ astro, page }) => { + await page.goto('/'); + + const counter = await page.locator('#react-counter'); + await expect(counter, 'component is visible').toBeVisible(); + + const count = await counter.locator('#react-counter-count'); + await expect(count, 'initial count is 0').toHaveText('0'); + + const increment = await counter.locator('#react-counter-increment'); + await increment.click(); + + await expect(count, 'count incremented by 1').toHaveText('1'); + }); + + test('Preact counter', async ({ astro, page }) => { + await page.goto('/'); + + const counter = await page.locator('#preact-counter'); + await expect(counter, 'component is visible').toBeVisible(); + + const count = await counter.locator('#preact-counter-count'); + await expect(count, 'initial count is 0').toHaveText('0'); + + const increment = await counter.locator('#preact-counter-increment'); + await increment.click(); + + await expect(count, 'count incremented by 1').toHaveText('1'); + }); + + test('Solid counter', async ({ astro, page }) => { + await page.goto('/'); + + const counter = await page.locator('#solid-counter'); + await expect(counter, 'component is visible').toBeVisible(); + + const count = await counter.locator('#solid-counter-count'); + await expect(count, 'initial count is 0').toHaveText('0'); + + const increment = await counter.locator('#solid-counter-increment'); + await increment.click(); + + await expect(count, 'count incremented by 1').toHaveText('1'); + }); + + test('Vue counter', async ({ astro, page }) => { + await page.goto('/'); + + const counter = await page.locator('#vue-counter'); + await expect(counter, 'component is visible').toBeVisible(); + + const count = await counter.locator('#vue-counter-count'); + await expect(count, 'initial count is 0').toHaveText('0'); + + const increment = await counter.locator('#vue-counter-increment'); + await increment.click(); + + await expect(count, 'count incremented by 1').toHaveText('1'); + }); + + test('Svelte counter', async ({ astro, page }) => { + await page.goto('/'); + + const counter = await page.locator('#svelte-counter'); + await expect(counter, 'component is visible').toBeVisible(); + + const count = await counter.locator('#svelte-counter-count'); + await expect(count, 'initial count is 0').toHaveText('0'); + + const increment = await counter.locator('#svelte-counter-increment'); + await increment.click(); + + await expect(count, 'count incremented by 1').toHaveText('1'); + }); +}); diff --git a/src/@types/astro.ts b/src/@types/astro.ts index dc2af7db33a7..d2ef923654f6 100644 --- a/src/@types/astro.ts +++ b/src/@types/astro.ts @@ -737,7 +737,7 @@ export interface AstroConfig extends z.output { export type AsyncRendererComponentFn = ( Component: any, props: any, - children: string | undefined, + slots: Record, metadata?: AstroComponentMetadata ) => Promise; diff --git a/src/runtime/server/astro-island.ts b/src/runtime/server/astro-island.ts index a067c9a5703e..10c69aa8e66b 100644 --- a/src/runtime/server/astro-island.ts +++ b/src/runtime/server/astro-island.ts @@ -64,23 +64,24 @@ declare const Astro: { if (!this.hydrator || this.parentElement?.closest('astro-island[ssr]')) { return; } - let innerHTML: string | null = null; - let fragment = this.querySelector('astro-fragment'); - if (fragment == null && this.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 = this.querySelector('template[data-astro-template]'); - if (template) { - innerHTML = template.innerHTML; - template.remove(); - } - } else if (fragment) { - innerHTML = fragment.innerHTML; + const slotted = this.querySelectorAll('astro-slot'); + const slots: Record = {}; + // Always check to see if there are templates. + // This happens if slots were passed but the client component did not render them. + const templates = this.querySelectorAll('template[data-astro-template]'); + for (const template of templates) { + if (!template.closest(this.tagName)?.isSameNode(this)) continue; + slots[template.getAttribute('data-astro-template') || 'default'] = template.innerHTML; + template.remove(); + } + for (const slot of slotted) { + if (!slot.closest(this.tagName)?.isSameNode(this)) continue; + slots[slot.getAttribute('name') || 'default'] = slot.innerHTML; } const props = this.hasAttribute('props') ? JSON.parse(this.getAttribute('props')!, reviver) : {}; - this.hydrator(this)(this.Component, props, innerHTML, { + this.hydrator(this)(this.Component, props, slots, { client: this.getAttribute('client'), }); this.removeAttribute('ssr'); diff --git a/src/runtime/server/index.ts b/src/runtime/server/index.ts index 6c7b12699369..1b78d7171332 100644 --- a/src/runtime/server/index.ts +++ b/src/runtime/server/index.ts @@ -208,7 +208,16 @@ Did you mean to add ${formatList(probableRendererNames.map((r) => '`' + r + '`') throw new Error(message); } - const children = await renderSlot(result, slots?.default); + const children: Record = {}; + if (slots) { + await Promise.all( + Object.entries(slots).map(([key, value]) => + renderSlot(result, value as string).then((output) => { + children[key] = output; + }) + ) + ); + } // Call the renderers `check` hook to see if any claim this component. let renderer: SSRLoadedRenderer | undefined; if (metadata.hydrate !== 'only') { @@ -307,11 +316,12 @@ If you're still stuck, please open an issue on GitHub or join us at https://astr // This is a custom element without a renderer. Because of that, render it // as a string and the user is responsible for adding a script tag for the component definition. if (!html && typeof Component === 'string') { + const childSlots = Object.values(children).join(''); html = await renderAstroComponent( await render`<${Component}${internalSpreadAttributes(props)}${markHTMLString( - (children == null || children == '') && voidElementNames.test(Component) + childSlots === '' && voidElementNames.test(Component) ? `/>` - : `>${children == null ? '' : children}` + : `>${childSlots}` )}` ); } @@ -320,7 +330,7 @@ If you're still stuck, please open an issue on GitHub or join us at https://astr if (isPage) { return html; } - return markHTMLString(html.replace(/\<\/?astro-fragment\>/g, '')); + return markHTMLString(html.replace(/\<\/?astro-slot\>/g, '')); } // Include componentExport name, componentUrl, and props in hash to dedupe identical islands @@ -336,13 +346,30 @@ If you're still stuck, please open an issue on GitHub or join us at https://astr ); result._metadata.needsHydrationStyles = true; - // Render a template if no fragment is provided. - const needsAstroTemplate = children && !/<\/?astro-fragment\>/.test(html); - const template = needsAstroTemplate ? `` : ''; - - if (needsAstroTemplate) { - island.props.tmpl = ''; - } + // Render template if not all astro fragments are provided. + let unrenderedSlots: string[] = []; + if (html) { + if (Object.keys(children).length > 0) { + for (const key of Object.keys(children)) { + if (!html.includes(key === 'default' ? `` : ``)) { + unrenderedSlots.push(key); + } + } + } + } else { + unrenderedSlots = Object.keys(children); + } + const template = + unrenderedSlots.length > 0 + ? unrenderedSlots + .map( + (key) => + `` + ) + .join('') + : ''; island.children = `${html ?? ''}${template}`; @@ -652,7 +679,7 @@ export async function renderHead(result: SSRResult): Promise { styles.push( renderElement('style', { props: {}, - children: 'astro-island, astro-fragment { display: contents; }', + children: 'astro-island, astro-slot { display: contents; }', }) ); } diff --git a/test/fixtures/lit-element/src/components/my-element.js b/test/fixtures/lit-element/src/components/my-element.js index b2cf72dea8b3..e946924cf7ff 100644 --- a/test/fixtures/lit-element/src/components/my-element.js +++ b/test/fixtures/lit-element/src/components/my-element.js @@ -29,6 +29,10 @@ export class MyElement extends LitElement {
${this.str}
data: ${this.obj.data}
${typeofwindow}
+ + +
+
`; } } diff --git a/test/fixtures/lit-element/src/pages/slots.astro b/test/fixtures/lit-element/src/pages/slots.astro new file mode 100644 index 000000000000..b8fc4963c6f0 --- /dev/null +++ b/test/fixtures/lit-element/src/pages/slots.astro @@ -0,0 +1,15 @@ +--- +import {MyElement} from '../components/my-element.js'; +--- + + + + LitElement | Slot + + + +
default
+
named
+ + + diff --git a/test/fixtures/slots-preact/src/components/Counter.jsx b/test/fixtures/slots-preact/src/components/Counter.jsx index cc11b9ee3fd4..16d2a95b948b 100644 --- a/test/fixtures/slots-preact/src/components/Counter.jsx +++ b/test/fixtures/slots-preact/src/components/Counter.jsx @@ -1,7 +1,7 @@ import { h, Fragment } from 'preact'; import { useState } from 'preact/hooks' -export default function Counter({ children, count: initialCount, case: id }) { +export default function Counter({ named, dashCase, children, count: initialCount, case: id }) { const [count, setCount] = useState(initialCount); const add = () => setCount((i) => i + 1); const subtract = () => setCount((i) => i - 1); @@ -15,6 +15,8 @@ export default function Counter({ children, count: initialCount, case: id }) {
{children ||

Fallback

} + {named} + {dashCase}
); diff --git a/test/fixtures/slots-preact/src/pages/index.astro b/test/fixtures/slots-preact/src/pages/index.astro index f8f101e73ecf..b2b039566b2d 100644 --- a/test/fixtures/slots-preact/src/pages/index.astro +++ b/test/fixtures/slots-preact/src/pages/index.astro @@ -8,4 +8,6 @@ import Counter from '../components/Counter.jsx' {false} {''}

Hello world!

 +

/ Named

 +

/ Dash Case

 diff --git a/test/fixtures/slots-preact/src/pages/markdown.md b/test/fixtures/slots-preact/src/pages/markdown.md new file mode 100644 index 000000000000..f86720fea9e0 --- /dev/null +++ b/test/fixtures/slots-preact/src/pages/markdown.md @@ -0,0 +1,9 @@ +--- +setup: import Counter from '../components/Counter.jsx' +--- + +# Slots: Preact + +

Hello world!

 +

/ Named

 +

/ Dash Case

 diff --git a/test/fixtures/slots-react/src/components/Counter.jsx b/test/fixtures/slots-react/src/components/Counter.jsx index 93f267ca4ffb..733cc47cc050 100644 --- a/test/fixtures/slots-react/src/components/Counter.jsx +++ b/test/fixtures/slots-react/src/components/Counter.jsx @@ -1,6 +1,6 @@ import React, { useState } from 'react'; -export default function Counter({ children, count: initialCount, case: id }) { +export default function Counter({ named, dashCase, children, count: initialCount, case: id }) { const [count, setCount] = useState(initialCount); const add = () => setCount((i) => i + 1); const subtract = () => setCount((i) => i - 1); @@ -14,6 +14,8 @@ export default function Counter({ children, count: initialCount, case: id }) {
{children ||

Fallback

} + {named} + {dashCase}
); diff --git a/test/fixtures/slots-react/src/pages/index.astro b/test/fixtures/slots-react/src/pages/index.astro index f8f101e73ecf..b2b039566b2d 100644 --- a/test/fixtures/slots-react/src/pages/index.astro +++ b/test/fixtures/slots-react/src/pages/index.astro @@ -8,4 +8,6 @@ import Counter from '../components/Counter.jsx' {false} {''}

Hello world!

 +

/ Named

 +

/ Dash Case

 diff --git a/test/fixtures/slots-react/src/pages/markdown.md b/test/fixtures/slots-react/src/pages/markdown.md new file mode 100644 index 000000000000..308450506164 --- /dev/null +++ b/test/fixtures/slots-react/src/pages/markdown.md @@ -0,0 +1,9 @@ +--- +setup: import Counter from '../components/Counter.jsx' +--- + +# Slots: React + +

Hello world!

 +

/ Named

 +

/ Dash Case

 diff --git a/test/fixtures/slots-solid/src/components/Counter.jsx b/test/fixtures/slots-solid/src/components/Counter.jsx index 6a585b8e3c4e..4b9c63c66722 100644 --- a/test/fixtures/slots-solid/src/components/Counter.jsx +++ b/test/fixtures/slots-solid/src/components/Counter.jsx @@ -1,6 +1,6 @@ import { createSignal } from 'solid-js'; -export default function Counter({ children, count: initialCount, case: id }) { +export default function Counter({ named, dashCase, children, count: initialCount, case: id }) { const [count] = createSignal(0); return ( <> @@ -9,6 +9,8 @@ export default function Counter({ children, count: initialCount, case: id }) {
{children ||

Fallback

} + {named} + {dashCase}
); diff --git a/test/fixtures/slots-solid/src/pages/index.astro b/test/fixtures/slots-solid/src/pages/index.astro index f8f101e73ecf..b2b039566b2d 100644 --- a/test/fixtures/slots-solid/src/pages/index.astro +++ b/test/fixtures/slots-solid/src/pages/index.astro @@ -8,4 +8,6 @@ import Counter from '../components/Counter.jsx' {false} {''}

Hello world!

 +

/ Named

 +

/ Dash Case

 diff --git a/test/fixtures/slots-solid/src/pages/markdown.md b/test/fixtures/slots-solid/src/pages/markdown.md new file mode 100644 index 000000000000..d9bc2dabd07a --- /dev/null +++ b/test/fixtures/slots-solid/src/pages/markdown.md @@ -0,0 +1,9 @@ +--- +setup: import Counter from '../components/Counter.jsx' +--- + +# Slots: Solid + +

Hello world!

 +

/ Named

 +

/ Dash Case

 diff --git a/test/fixtures/slots-svelte/src/components/Counter.svelte b/test/fixtures/slots-svelte/src/components/Counter.svelte index 11901049e924..24f4e734e658 100644 --- a/test/fixtures/slots-svelte/src/components/Counter.svelte +++ b/test/fixtures/slots-svelte/src/components/Counter.svelte @@ -17,9 +17,7 @@
- -

Fallback

- +

Fallback