From 29cdfa024886dd581cb207586f7dfec6966bdd4e Mon Sep 17 00:00:00 2001 From: Martin Trapp <94928215+martrapp@users.noreply.github.com> Date: Mon, 9 Oct 2023 16:19:46 +0200 Subject: [PATCH] Fix transition attributes on islands (#8776) * Fix transition attributes on islands * Incorporate comments from review --- .changeset/wise-lions-lay.md | 5 +++++ .../astro/src/runtime/server/hydration.ts | 15 ++++++++++++--- .../src/runtime/server/render/component.ts | 6 +++--- .../view-transitions/astro.config.mjs | 7 +++++++ .../fixtures/view-transitions/package.json | 5 ++++- .../src/components/Island.css | 11 +++++++++++ .../src/components/Island.jsx | 19 +++++++++++++++++++ .../src/pages/hasIsland.astro | 10 ++++++++++ packages/astro/test/view-transitions.test.js | 10 ++++++++++ pnpm-lock.yaml | 9 +++++++++ 10 files changed, 90 insertions(+), 7 deletions(-) create mode 100644 .changeset/wise-lions-lay.md create mode 100644 packages/astro/test/fixtures/view-transitions/astro.config.mjs create mode 100644 packages/astro/test/fixtures/view-transitions/src/components/Island.css create mode 100644 packages/astro/test/fixtures/view-transitions/src/components/Island.jsx create mode 100644 packages/astro/test/fixtures/view-transitions/src/pages/hasIsland.astro diff --git a/.changeset/wise-lions-lay.md b/.changeset/wise-lions-lay.md new file mode 100644 index 000000000..5718740fe --- /dev/null +++ b/.changeset/wise-lions-lay.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Fix transition attributes on islands diff --git a/packages/astro/src/runtime/server/hydration.ts b/packages/astro/src/runtime/server/hydration.ts index 09f42a9b5..8c5635105 100644 --- a/packages/astro/src/runtime/server/hydration.ts +++ b/packages/astro/src/runtime/server/hydration.ts @@ -12,13 +12,16 @@ export interface HydrationMetadata { directive: string; value: string; componentUrl: string; - componentExport: { value: string }; + componentExport: { value: string; }; } +type Props = Record; + interface ExtractedProps { isPage: boolean; hydration: HydrationMetadata | null; - props: Record; + props: Props; + propsWithoutTransitionAttributes: Props; } const transitionDirectivesToCopyOnIsland = Object.freeze([ @@ -26,16 +29,18 @@ const transitionDirectivesToCopyOnIsland = Object.freeze([ 'data-astro-transition-persist', ]); + // Used to extract the directives, aka `client:load` information about a component. // Finds these special props and removes them from what gets passed into the component. export function extractDirectives( - inputProps: Record, + inputProps: Props, clientDirectives: SSRResult['clientDirectives'] ): ExtractedProps { let extracted: ExtractedProps = { isPage: false, hydration: null, props: {}, + propsWithoutTransitionAttributes: {}, }; for (const [key, value] of Object.entries(inputProps)) { if (key.startsWith('server:')) { @@ -96,10 +101,14 @@ export function extractDirectives( } } else { extracted.props[key] = value; + if (!transitionDirectivesToCopyOnIsland.includes(key)) { + extracted.propsWithoutTransitionAttributes[key] = value; + } } } for (const sym of Object.getOwnPropertySymbols(inputProps)) { extracted.props[sym] = inputProps[sym]; + extracted.propsWithoutTransitionAttributes[sym] = inputProps[sym]; } return extracted; diff --git a/packages/astro/src/runtime/server/render/component.ts b/packages/astro/src/runtime/server/render/component.ts index 158a88a07..91d74e812 100644 --- a/packages/astro/src/runtime/server/render/component.ts +++ b/packages/astro/src/runtime/server/render/component.ts @@ -92,7 +92,7 @@ async function renderFrameworkComponent( displayName, }; - const { hydration, isPage, props } = extractDirectives(_props, clientDirectives); + const { hydration, isPage, props, propsWithoutTransitionAttributes } = extractDirectives(_props, clientDirectives); let html = ''; let attrs: Record | undefined = undefined; @@ -217,7 +217,7 @@ async function renderFrameworkComponent( ({ html, attrs } = await renderer.ssr.renderToStaticMarkup.call( { result }, Component, - props, + propsWithoutTransitionAttributes, children, metadata )); @@ -242,7 +242,7 @@ If you're still stuck, please open an issue on GitHub or join us at https://astr ({ html, attrs } = await renderer.ssr.renderToStaticMarkup.call( { result }, Component, - props, + propsWithoutTransitionAttributes, children, metadata )); diff --git a/packages/astro/test/fixtures/view-transitions/astro.config.mjs b/packages/astro/test/fixtures/view-transitions/astro.config.mjs new file mode 100644 index 000000000..8a6f1951c --- /dev/null +++ b/packages/astro/test/fixtures/view-transitions/astro.config.mjs @@ -0,0 +1,7 @@ +import { defineConfig } from 'astro/config'; +import react from '@astrojs/react'; + +// https://astro.build/config +export default defineConfig({ + integrations: [react()], +}); diff --git a/packages/astro/test/fixtures/view-transitions/package.json b/packages/astro/test/fixtures/view-transitions/package.json index bc6790df4..41cee5081 100644 --- a/packages/astro/test/fixtures/view-transitions/package.json +++ b/packages/astro/test/fixtures/view-transitions/package.json @@ -3,6 +3,9 @@ "version": "0.0.0", "private": true, "dependencies": { - "astro": "workspace:*" + "astro": "workspace:*", + "@astrojs/react": "workspace:*", + "react": "^18.1.0", + "react-dom": "^18.1.0" } } diff --git a/packages/astro/test/fixtures/view-transitions/src/components/Island.css b/packages/astro/test/fixtures/view-transitions/src/components/Island.css new file mode 100644 index 000000000..fb21044d7 --- /dev/null +++ b/packages/astro/test/fixtures/view-transitions/src/components/Island.css @@ -0,0 +1,11 @@ +.counter { + display: grid; + font-size: 2em; + grid-template-columns: repeat(3, minmax(0, 1fr)); + margin-top: 2em; + place-items: center; +} + +.counter-message { + text-align: center; +} diff --git a/packages/astro/test/fixtures/view-transitions/src/components/Island.jsx b/packages/astro/test/fixtures/view-transitions/src/components/Island.jsx new file mode 100644 index 000000000..cde384980 --- /dev/null +++ b/packages/astro/test/fixtures/view-transitions/src/components/Island.jsx @@ -0,0 +1,19 @@ +import React, { useState } from 'react'; +import './Island.css'; + +export default function Counter({ children, count: initialCount, id }) { + const [count, setCount] = useState(initialCount); + const add = () => setCount((i) => i + 1); + const subtract = () => setCount((i) => i - 1); + + return ( + <> +
+ + 
{count}
+ +
+
{children}
+ + ); +} diff --git a/packages/astro/test/fixtures/view-transitions/src/pages/hasIsland.astro b/packages/astro/test/fixtures/view-transitions/src/pages/hasIsland.astro new file mode 100644 index 000000000..378afb180 --- /dev/null +++ b/packages/astro/test/fixtures/view-transitions/src/pages/hasIsland.astro @@ -0,0 +1,10 @@ +--- +import Island from '../components/Island.jsx'; +--- + + + + + + + diff --git a/packages/astro/test/view-transitions.test.js b/packages/astro/test/view-transitions.test.js index 6d4d19cbb..008a6e09f 100644 --- a/packages/astro/test/view-transitions.test.js +++ b/packages/astro/test/view-transitions.test.js @@ -22,4 +22,14 @@ describe('View Transitions styles', () => { expect($('head style')).to.have.a.lengthOf(3); }); + + it('should not duplicate transition attributes on island contents', async () => { + let res = await fixture.fetch('/hasIsland'); + let html = await res.text(); + let $ = cheerio.load(html); + expect($('astro-island[data-astro-transition-persist]')).to.have.a.lengthOf(1); + expect( + $('astro-island[data-astro-transition-persist] > [data-astro-transition-persist]') + ).to.have.a.lengthOf(0); + }); }); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index d70075993..c39875566 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -3531,9 +3531,18 @@ importers: packages/astro/test/fixtures/view-transitions: dependencies: + '@astrojs/react': + specifier: workspace:* + version: link:../../../../integrations/react astro: specifier: workspace:* version: link:../../.. + react: + specifier: ^18.1.0 + version: 18.2.0 + react-dom: + specifier: ^18.1.0 + version: 18.2.0(react@18.2.0) packages/astro/test/fixtures/virtual-astro-file: dependencies: