From 5c888c10b712ca60a23e66b88af8051b54b42323 Mon Sep 17 00:00:00 2001 From: Martin Trapp <94928215+martrapp@users.noreply.github.com> Date: Thu, 19 Oct 2023 21:55:22 +0200 Subject: [PATCH] Fix: Persist styles of persistent client:only components during view transitions in DEV mode (#8840) * 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 --------- Co-authored-by: Sarah Rainsberger --- .changeset/purple-dots-refuse.md | 5 ++ .../view-transitions/astro.config.mjs | 4 +- .../fixtures/view-transitions/package.json | 4 ++ .../src/components/Island.css | 2 + .../src/components/Island.jsx | 1 + .../src/components/SvelteCounter.svelte | 36 +++++++++++ .../src/components/VueCounter.vue | 43 +++++++++++++ .../view-transitions/src/components/css.js | 3 + .../src/components/other.postcss | 1 + .../src/pages/client-only-four.astro | 11 ++++ .../src/pages/client-only-one.astro | 2 +- .../src/pages/client-only-three.astro | 16 +++++ .../src/pages/client-only-two.astro | 2 +- packages/astro/e2e/view-transitions.test.js | 32 ++++++++-- packages/astro/src/transitions/router.ts | 64 +++++++++++++++++-- pnpm-lock.yaml | 12 ++++ 16 files changed, 225 insertions(+), 13 deletions(-) create mode 100644 .changeset/purple-dots-refuse.md create mode 100644 packages/astro/e2e/fixtures/view-transitions/src/components/SvelteCounter.svelte create mode 100644 packages/astro/e2e/fixtures/view-transitions/src/components/VueCounter.vue create mode 100644 packages/astro/e2e/fixtures/view-transitions/src/components/css.js create mode 100644 packages/astro/e2e/fixtures/view-transitions/src/components/other.postcss create mode 100644 packages/astro/e2e/fixtures/view-transitions/src/pages/client-only-four.astro create mode 100644 packages/astro/e2e/fixtures/view-transitions/src/pages/client-only-three.astro diff --git a/.changeset/purple-dots-refuse.md b/.changeset/purple-dots-refuse.md new file mode 100644 index 000000000000..74be758b57f9 --- /dev/null +++ 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 index 2b22ff9cf3f3..f4450f67285d 100644 --- 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 index f4ba9b17b053..b53b5fcad4a6 100644 --- 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 index fb21044d78cc..28c5642a9897 100644 --- 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 index cde38498028b..734e2011b25b 100644 --- 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 new file mode 100644 index 000000000000..6647a19ce7d0 --- /dev/null +++ b/packages/astro/e2e/fixtures/view-transitions/src/components/SvelteCounter.svelte @@ -0,0 +1,36 @@ + + +
+ + 
{count}
+ +
+
+ +
+ + diff --git a/packages/astro/e2e/fixtures/view-transitions/src/components/VueCounter.vue b/packages/astro/e2e/fixtures/view-transitions/src/components/VueCounter.vue new file mode 100644 index 000000000000..e75620aff455 --- /dev/null +++ b/packages/astro/e2e/fixtures/view-transitions/src/components/VueCounter.vue @@ -0,0 +1,43 @@ + + + + + diff --git a/packages/astro/e2e/fixtures/view-transitions/src/components/css.js b/packages/astro/e2e/fixtures/view-transitions/src/components/css.js new file mode 100644 index 000000000000..b2bf4b9679c4 --- /dev/null +++ b/packages/astro/e2e/fixtures/view-transitions/src/components/css.js @@ -0,0 +1,3 @@ +import "./other.postcss"; +export const indirect = ""; + diff --git a/packages/astro/e2e/fixtures/view-transitions/src/components/other.postcss b/packages/astro/e2e/fixtures/view-transitions/src/components/other.postcss new file mode 100644 index 000000000000..55b21b9202f2 --- /dev/null +++ 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 new file mode 100644 index 000000000000..9ebfa65f04e8 --- /dev/null +++ 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'; +--- + +

Page 4

+ Vue + Svelte + 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 index a8d5e8995ae4..a51ccc299b2a 100644 --- 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'; go to page 2
- message here + message here
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 new file mode 100644 index 000000000000..34fa6992699b --- /dev/null +++ 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'; +--- + + go to page 4 +
+ + message here +
+ Vue + Svelte +

client-only-three

+ 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 index 884ec46833d5..4190d86efb45 100644 --- 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';

Page 2

- message here + message here
diff --git a/packages/astro/e2e/view-transitions.test.js b/packages/astro/e2e/view-transitions.test.js index d2c14aabdabc..a378df7c530c 100644 --- 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 index 869ed87af5fa..1bbbc85a138d 100644 --- 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(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 index 6116f2c0c661..a19fcf7c8bc3 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1493,6 +1493,12 @@ importers: '@astrojs/react': specifier: workspace:* version: link:../../../../integrations/react + '@astrojs/svelte': + specifier: workspace:* + version: link:../../../../integrations/svelte + '@astrojs/vue': + specifier: workspace:* + version: link:../../../../integrations/vue astro: specifier: workspace:* version: link:../../.. @@ -1502,6 +1508,12 @@ importers: react-dom: specifier: ^18.1.0 version: 18.2.0(react@18.2.0) + svelte: + specifier: ^4.2.0 + version: 4.2.0 + vue: + specifier: ^3.3.4 + version: 3.3.4 packages/astro/e2e/fixtures/vue-component: dependencies: