Fix: Persist styles of persistent client:only components during view transitions in DEV mode (#8840)

martrapp · sarah11918 · web-flow · commit 5c888c10b712 · 2023-10-19T21:55:22.000+02:00
* Persist styles of persistent client:only components during view transitions

* Persist styles of persistent client:only components during view transitions

* Persist styles of persistent client:only components during view transitions

* reset flag for persistent style shhets before re-calculating

* new approach with a clear module loader cache

* simplifications

* wait for hydration

* improve changeset message

* improve changeset message

* please the linter

* additional tests for Svelte and Vue

* tidy up

* test fixed

* test w/o persistence

* Update .changeset/purple-dots-refuse.md

Co-authored-by: Sarah Rainsberger &lt;sarah@rainsberger.ca&gt;

---------

Co-authored-by: Sarah Rainsberger &lt;sarah@rainsberger.ca&gt;
diff --git a/.changeset/purple-dots-refuse.md b/.changeset/purple-dots-refuse.md
@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Fixes styles of `client:only` components not persisting during view transitions in dev mode
diff --git a/packages/astro/e2e/fixtures/view-transitions/astro.config.mjs b/packages/astro/e2e/fixtures/view-transitions/astro.config.mjs
@@ -1,12 +1,14 @@
 import { defineConfig } from 'astro/config';
 import react from '@astrojs/react';
+import vue from '@astrojs/vue';
+import svelte from '@astrojs/svelte';
 import nodejs from '@astrojs/node';
 
 // https://astro.build/config
 export default defineConfig({
 	output: 'hybrid',
 	adapter: nodejs({ mode: 'standalone' }),
-	integrations: [react()],
+	integrations: [react(),vue(),svelte()],
 	redirects: {
 		'/redirect-two': '/two',
 		'/redirect-external': 'http://example.com/',
diff --git a/packages/astro/e2e/fixtures/view-transitions/package.json b/packages/astro/e2e/fixtures/view-transitions/package.json
@@ -6,6 +6,10 @@
     "astro": "workspace:*",
     "@astrojs/node": "workspace:*",
     "@astrojs/react": "workspace:*",
+    "@astrojs/vue": "workspace:*",
+    "@astrojs/svelte": "workspace:*",
+    "svelte": "^4.2.0",
+    "vue": "^3.3.4",
     "react": "^18.1.0",
     "react-dom": "^18.1.0"
   }
diff --git a/packages/astro/e2e/fixtures/view-transitions/src/components/Island.css b/packages/astro/e2e/fixtures/view-transitions/src/components/Island.css
@@ -8,4 +8,6 @@
 
 .counter-message {
 	text-align: center;
+	background-color: lightskyblue;
+	color:black
 }
diff --git a/packages/astro/e2e/fixtures/view-transitions/src/components/Island.jsx b/packages/astro/e2e/fixtures/view-transitions/src/components/Island.jsx
@@ -1,5 +1,6 @@
 import React, { useState } from 'react';
 import './Island.css';
+import { indirect} from './css.js';
 
 export default function Counter({ children, count: initialCount, id }) {
 	const [count, setCount] = useState(initialCount);
diff --git a/packages/astro/e2e/fixtures/view-transitions/src/components/SvelteCounter.svelte b/packages/astro/e2e/fixtures/view-transitions/src/components/SvelteCounter.svelte
@@ -0,0 +1,36 @@
+<script lang="ts">
+	let count = 0;
+
+	function add() {
+		count += 1;
+	}
+
+	function subtract() {
+		count -= 1;
+	}
+</script>
+
+<div class="counter">
+	<button on:click={subtract}>-</button>
+	<pre>{count}</pre>
+	<button on:click={add}>+</button>
+</div>
+<div class="message">
+	<slot />
+</div>
+
+<style>
+	.counter {
+		display: grid;
+		font-size: 2em;
+		grid-template-columns: repeat(3, minmax(0, 1fr));
+		margin-top: 2em;
+		place-items: center;
+	}
+
+	.message {
+		text-align: center;
+		background-color: maroon;
+		color: tomato;
+	}
+</style>
diff --git a/packages/astro/e2e/fixtures/view-transitions/src/components/VueCounter.vue b/packages/astro/e2e/fixtures/view-transitions/src/components/VueCounter.vue
@@ -0,0 +1,43 @@
+<template>
+	<div class="counter">
+		<button @click="subtract()">-</button>
+		<pre>{{ count }}</pre>
+		<button @click="add()">+</button>
+	</div>
+	<div class="counter-message">
+		<slot />
+	</div>
+</template>
+
+<script lang="ts">
+import { ref } from 'vue';
+export default {
+	setup() {
+		const count = ref(0);
+		const add = () => (count.value = count.value + 1);
+		const subtract = () => (count.value = count.value - 1);
+
+		return {
+			count,
+			add,
+			subtract,
+		};
+	},
+};
+</script>
+
+<style scoped>
+.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;
+	background: darkgreen;
+	color: greenyellow;
+}
+</style>
diff --git a/packages/astro/e2e/fixtures/view-transitions/src/components/css.js b/packages/astro/e2e/fixtures/view-transitions/src/components/css.js
@@ -0,0 +1,3 @@
+import "./other.postcss";
+export const indirect = "<dummy>";
+
diff --git a/packages/astro/e2e/fixtures/view-transitions/src/components/other.postcss b/packages/astro/e2e/fixtures/view-transitions/src/components/other.postcss
@@ -0,0 +1 @@
+/* not much to see */
diff --git a/packages/astro/e2e/fixtures/view-transitions/src/pages/client-only-four.astro b/packages/astro/e2e/fixtures/view-transitions/src/pages/client-only-four.astro
@@ -0,0 +1,11 @@
+---
+import Layout from '../components/Layout.astro';
+import Island from '../components/Island';
+import VueCounter from '../components/VueCounter.vue';
+import SvelteCounter from '../components/SvelteCounter.svelte';
+---
+<Layout>
+	<p id="page-four">Page 4</p>
+		<VueCounter client:only="vue">Vue</VueCounter>
+		<SvelteCounter client:only="svelte">Svelte</SvelteCounter>
+</Layout>
diff --git a/packages/astro/e2e/fixtures/view-transitions/src/pages/client-only-one.astro b/packages/astro/e2e/fixtures/view-transitions/src/pages/client-only-one.astro
@@ -5,6 +5,6 @@ import Island from '../components/Island';
 <Layout>
 	<a id="click-two" href="/client-only-two">go to page 2</a>
 	<div transition:persist="island">
-		<Island client:only count={5}>message here</Island>
+		<Island id="one" client:only="react" count={5}>message here</Island>
 	</div>
 </Layout>
diff --git a/packages/astro/e2e/fixtures/view-transitions/src/pages/client-only-three.astro b/packages/astro/e2e/fixtures/view-transitions/src/pages/client-only-three.astro
@@ -0,0 +1,16 @@
+---
+import Layout from '../components/Layout.astro';
+import Island from '../components/Island';
+import VueCounter from '../components/VueCounter.vue';
+import SvelteCounter from '../components/SvelteCounter.svelte';
+---
+<Layout>
+	<a id="click-four" href="/client-only-four">go to page 4</a>
+	<div>
+		<!-- intentional client:load, see /client-only-one for client:only -->
+		<Island id="one" client:load count={5}>message here</Island>
+	</div>
+		<VueCounter client:only="vue">Vue</VueCounter>
+		<SvelteCounter client:only="svelte">Svelte</SvelteCounter>
+	<p id="name">client-only-three</p>
+</Layout>
diff --git a/packages/astro/e2e/fixtures/view-transitions/src/pages/client-only-two.astro b/packages/astro/e2e/fixtures/view-transitions/src/pages/client-only-two.astro
@@ -5,6 +5,6 @@ import Island from '../components/Island';
 <Layout>
 	<p id="page-two">Page 2</p>
 	<div transition:persist="island">
-		<Island client:only count={5}>message here</Island>
+		<Island id="two" client:only="react" count={5}>message here</Island>
 	</div>
 </Layout>
diff --git a/packages/astro/e2e/view-transitions.test.js b/packages/astro/e2e/view-transitions.test.js
@@ -241,15 +241,15 @@ test.describe('View Transitions', () => {
 		let p = page.locator('#totwo');
 		await expect(p, 'should have content').toHaveText('Go to listener two');
 		// on load a CSS transition is started triggered by a class on the html element
-		expect(transitions).toEqual(1);
-
+		expect(transitions).toBeLessThanOrEqual(1);
+		const transitionsBefore = transitions;
 		// go to page 2
 		await page.click('#totwo');
 		p = page.locator('#toone');
 		await expect(p, 'should have content').toHaveText('Go to listener one');
 		// swap() resets that class, the after-swap listener sets it again.
 		// the temporarily missing class must not trigger page rendering
-		expect(transitions).toEqual(1);
+		expect(transitions).toEqual(transitionsBefore);
 	});
 
 	test('click hash links does not do navigation', async ({ page, astro }) => {
@@ -670,10 +670,9 @@ test.describe('View Transitions', () => {
 		expect(loads.length, 'There should be 2 page loads').toEqual(2);
 	});
 
-	test.skip('client:only styles are retained on transition', async ({ page, astro }) => {
-		const totalExpectedStyles = 7;
+	test('client:only styles are retained on transition (1/2)', async ({ page, astro }) => {
+		const totalExpectedStyles = 8;
 
-		// Go to page 1
 		await page.goto(astro.resolveUrl('/client-only-one'));
 		let msg = page.locator('.counter-message');
 		await expect(msg).toHaveText('message here');
@@ -690,6 +689,27 @@ test.describe('View Transitions', () => {
 		expect(styles.length).toEqual(totalExpectedStyles, 'style count has not changed');
 	});
 
+	test('client:only styles are retained on transition (2/2)', async ({ page, astro }) => {
+		const totalExpectedStyles_page_three = 10;
+		const totalExpectedStyles_page_four = 8;
+
+		await page.goto(astro.resolveUrl('/client-only-three'));
+		let msg = page.locator('#name');
+		await expect(msg).toHaveText('client-only-three');
+		await page.waitForTimeout(400); // await hydration
+
+		let styles = await page.locator('style').all();
+		expect(styles.length).toEqual(totalExpectedStyles_page_three);
+
+		await page.click('#click-four');
+
+		let pageTwo = page.locator('#page-four');
+		await expect(pageTwo, 'should have content').toHaveText('Page 4');
+
+		styles = await page.locator('style').all();
+		expect(styles.length).toEqual(totalExpectedStyles_page_four, 'style count has not changed');
+	});
+
 	test('Horizontal scroll position restored on back button', async ({ page, astro }) => {
 		await page.goto(astro.resolveUrl('/wide-page'));
 		let article = page.locator('#widepage');
diff --git a/packages/astro/src/transitions/router.ts b/packages/astro/src/transitions/router.ts
@@ -47,6 +47,7 @@ const announce = () => {
 };
 
 const PERSIST_ATTR = 'data-astro-transition-persist';
+const VITE_ID = 'data-vite-dev-id';
 
 let parser: DOMParser;
 
@@ -202,8 +203,10 @@ async function updateDOM(
 ) {
 	// Check for a head element that should persist and returns it,
 	// either because it has the data attribute or is a link el.
-	const persistedHeadElement = (el: HTMLElement): Element | null => {
+	// Returns null if the element is not part of the new head, undefined if it should be left alone.
+	const persistedHeadElement = (el: HTMLElement): Element | null | undefined => {
 		const id = el.getAttribute(PERSIST_ATTR);
+		if (id === '') return undefined;
 		const newEl = id && newDocument.head.querySelector(`[${PERSIST_ATTR}="${id}"]`);
 		if (newEl) {
 			return newEl;
@@ -226,7 +229,7 @@ async function updateDOM(
 		// The element that currently has the focus is part of a DOM tree
 		// that will survive the transition to the new document.
 		// Save the element and the cursor position
-		if (activeElement?.closest('[data-astro-transition-persist]')) {
+		if (activeElement?.closest(`[${PERSIST_ATTR}]`)) {
 			if (
 				activeElement instanceof HTMLInputElement ||
 				activeElement instanceof HTMLTextAreaElement
@@ -290,7 +293,7 @@ async function updateDOM(
 			// from the new document and leave the current node alone
 			if (newEl) {
 				newEl.remove();
-			} else {
+			} else if (newEl === null) {
 				// Otherwise remove the element in the head. It doesn't exist in the new page.
 				el.remove();
 			}
@@ -306,6 +309,7 @@ async function updateDOM(
 
 		// this will reset scroll Position
 		document.body.replaceWith(newDocument.body);
+
 		for (const el of oldBody.querySelectorAll(`[${PERSIST_ATTR}]`)) {
 			const id = el.getAttribute(PERSIST_ATTR);
 			const newEl = document.querySelector(`[${PERSIST_ATTR}="${id}"]`);
@@ -315,7 +319,6 @@ async function updateDOM(
 				newEl.replaceWith(el);
 			}
 		}
-
 		restoreFocus(savedFocus);
 
 		if (popState) {
@@ -404,6 +407,8 @@ async function transition(
 		return;
 	}
 
+	if (import.meta.env.DEV) await prepareForClientOnlyComponents(newDocument, toLocation);
+
 	if (!popState) {
 		// save the current scroll position before we change the DOM and transition to the new page
 		history.replaceState({ ...history.state, scrollX, scrollY }, '');
@@ -438,6 +443,7 @@ export function navigate(href: string, options?: Options) {
 				'The view transtions client API was called during a server side render. This may be unintentional as the navigate() function is expected to be called in response to user interactions. Please make sure that your usage is correct.'
 			);
 			warning.name = 'Warning';
+			// eslint-disable-next-line no-console
 			console.warn(warning);
 			navigateOnServerWarned = true;
 		}
@@ -519,3 +525,53 @@ if (inBrowser) {
 		markScriptsExec();
 	}
 }
+
+// Keep all styles that are potentially created by client:only components
+// and required on the next page
+async function prepareForClientOnlyComponents(newDocument: Document, toLocation: URL) {
+	// Any client:only component on the next page?
+	if (newDocument.body.querySelector(`astro-island[client='only']`)) {
+		// Load the next page with an empty module loader cache
+		const nextPage = document.createElement('iframe');
+		nextPage.setAttribute('src', toLocation.href);
+		nextPage.style.display = 'none';
+		document.body.append(nextPage);
+		await hydrationDone(nextPage);
+
+		const nextHead = nextPage.contentDocument?.head;
+		if (nextHead) {
+			// Clear former persist marks
+			document.head
+				.querySelectorAll(`style[${PERSIST_ATTR}=""]`)
+				.forEach((s) => s.removeAttribute(PERSIST_ATTR));
+
+			// Collect the vite ids of all styles present in the next head
+			const viteIds = [...nextHead.querySelectorAll(`style[${VITE_ID}]`)].map((style) =>
+				style.getAttribute(VITE_ID)
+			);
+			// Mark styles of the current head as persistent
+			// if they come from hydration and not from the newDocument
+			viteIds.forEach((id) => {
+				const style = document.head.querySelector(`style[${VITE_ID}="${id}"]`);
+				if (style && !newDocument.head.querySelector(`style[${VITE_ID}="${id}"]`)) {
+					style.setAttribute(PERSIST_ATTR, '');
+				}
+			});
+		}
+
+		// return a promise that resolves when all astro-islands are hydrated
+		async function hydrationDone(loadingPage: HTMLIFrameElement) {
+			await new Promise(
+				(r) => loadingPage.contentWindow?.addEventListener('load', r, { once: true })
+			);
+
+			return new Promise<void>(async (r) => {
+				for (let count = 0; count <= 20; ++count) {
+					if (!loadingPage.contentDocument!.body.querySelector('astro-island[ssr]')) break;
+					await new Promise((r2) => setTimeout(r2, 50));
+				}
+				r();
+			});
+		}
+	}
+}
diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml

-Original file line number
+Diff line change
@@ @@ -0,0 +1,5 @@ @@
 +---
 +'astro': patch
 +---
++
 +Fixes styles of `client:only` components not persisting during view transitions in dev mode
Original file line number	Diff line number	Diff line change
`@@ -8,4 +8,6 @@`
`8`	`8`
`9`	`9`	`.counter-message {`
`10`	`10`	`text-align: center;`
	`11`	`+ background-color: lightskyblue;`
	`12`	`+ color:black`
`11`	`13`	`}`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+import "./other.postcss";`
	`2`	`+export const indirect = "<dummy>";`
	`3`	`+`