Fix transition attributes on islands (#8776)

* Fix transition attributes on islands

* Incorporate comments from review
This commit is contained in:
Martin Trapp 2023-10-09 16:19:46 +02:00 committed by GitHub
parent c24f70d916
commit 29cdfa0248
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 90 additions and 7 deletions

View file

@ -0,0 +1,5 @@
---
'astro': patch
---
Fix transition attributes on islands

View file

@ -12,13 +12,16 @@ export interface HydrationMetadata {
directive: string; directive: string;
value: string; value: string;
componentUrl: string; componentUrl: string;
componentExport: { value: string }; componentExport: { value: string; };
} }
type Props = Record<string | number | symbol, any>;
interface ExtractedProps { interface ExtractedProps {
isPage: boolean; isPage: boolean;
hydration: HydrationMetadata | null; hydration: HydrationMetadata | null;
props: Record<string | number | symbol, any>; props: Props;
propsWithoutTransitionAttributes: Props;
} }
const transitionDirectivesToCopyOnIsland = Object.freeze([ const transitionDirectivesToCopyOnIsland = Object.freeze([
@ -26,16 +29,18 @@ const transitionDirectivesToCopyOnIsland = Object.freeze([
'data-astro-transition-persist', 'data-astro-transition-persist',
]); ]);
// Used to extract the directives, aka `client:load` information about a component. // 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. // Finds these special props and removes them from what gets passed into the component.
export function extractDirectives( export function extractDirectives(
inputProps: Record<string | number | symbol, any>, inputProps: Props,
clientDirectives: SSRResult['clientDirectives'] clientDirectives: SSRResult['clientDirectives']
): ExtractedProps { ): ExtractedProps {
let extracted: ExtractedProps = { let extracted: ExtractedProps = {
isPage: false, isPage: false,
hydration: null, hydration: null,
props: {}, props: {},
propsWithoutTransitionAttributes: {},
}; };
for (const [key, value] of Object.entries(inputProps)) { for (const [key, value] of Object.entries(inputProps)) {
if (key.startsWith('server:')) { if (key.startsWith('server:')) {
@ -96,10 +101,14 @@ export function extractDirectives(
} }
} else { } else {
extracted.props[key] = value; extracted.props[key] = value;
if (!transitionDirectivesToCopyOnIsland.includes(key)) {
extracted.propsWithoutTransitionAttributes[key] = value;
}
} }
} }
for (const sym of Object.getOwnPropertySymbols(inputProps)) { for (const sym of Object.getOwnPropertySymbols(inputProps)) {
extracted.props[sym] = inputProps[sym]; extracted.props[sym] = inputProps[sym];
extracted.propsWithoutTransitionAttributes[sym] = inputProps[sym];
} }
return extracted; return extracted;

View file

@ -92,7 +92,7 @@ async function renderFrameworkComponent(
displayName, displayName,
}; };
const { hydration, isPage, props } = extractDirectives(_props, clientDirectives); const { hydration, isPage, props, propsWithoutTransitionAttributes } = extractDirectives(_props, clientDirectives);
let html = ''; let html = '';
let attrs: Record<string, string> | undefined = undefined; let attrs: Record<string, string> | undefined = undefined;
@ -217,7 +217,7 @@ async function renderFrameworkComponent(
({ html, attrs } = await renderer.ssr.renderToStaticMarkup.call( ({ html, attrs } = await renderer.ssr.renderToStaticMarkup.call(
{ result }, { result },
Component, Component,
props, propsWithoutTransitionAttributes,
children, children,
metadata 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( ({ html, attrs } = await renderer.ssr.renderToStaticMarkup.call(
{ result }, { result },
Component, Component,
props, propsWithoutTransitionAttributes,
children, children,
metadata metadata
)); ));

View file

@ -0,0 +1,7 @@
import { defineConfig } from 'astro/config';
import react from '@astrojs/react';
// https://astro.build/config
export default defineConfig({
integrations: [react()],
});

View file

@ -3,6 +3,9 @@
"version": "0.0.0", "version": "0.0.0",
"private": true, "private": true,
"dependencies": { "dependencies": {
"astro": "workspace:*" "astro": "workspace:*",
"@astrojs/react": "workspace:*",
"react": "^18.1.0",
"react-dom": "^18.1.0"
} }
} }

View file

@ -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;
}

View file

@ -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 (
<>
<div id={id} className="counter">
<button className="decrement" onClick={subtract}>-</button>
<pre>{count}</pre>
<button className="increment" onClick={add}>+</button>
</div>
<div className="counter-message">{children}</div>
</>
);
}

View file

@ -0,0 +1,10 @@
---
import Island from '../components/Island.jsx';
---
<html>
<head>
</head>
<body>
<Island id="1" count="{1}" children="Greetings!" transition:persist="here" client:load/>
</body>
</html>

View file

@ -22,4 +22,14 @@ describe('View Transitions styles', () => {
expect($('head style')).to.have.a.lengthOf(3); 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);
});
}); });

View file

@ -3531,9 +3531,18 @@ importers:
packages/astro/test/fixtures/view-transitions: packages/astro/test/fixtures/view-transitions:
dependencies: dependencies:
'@astrojs/react':
specifier: workspace:*
version: link:../../../../integrations/react
astro: astro:
specifier: workspace:* specifier: workspace:*
version: link:../../.. 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: packages/astro/test/fixtures/virtual-astro-file:
dependencies: dependencies: