Skip to content

Commit

Permalink
Fix: Persist styles of persistent client:only components during view …
Browse files Browse the repository at this point in the history
…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 <sarah@rainsberger.ca>

---------

Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
  • Loading branch information
martrapp and sarah11918 authored Oct 19, 2023
1 parent 740c916 commit 5c888c1
Show file tree
Hide file tree
Showing 16 changed files with 225 additions and 13 deletions.
5 changes: 5 additions & 0 deletions .changeset/purple-dots-refuse.md
Original file line number Diff 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
@@ -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/',
Expand Down
4 changes: 4 additions & 0 deletions packages/astro/e2e/fixtures/view-transitions/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,6 @@

.counter-message {
text-align: center;
background-color: lightskyblue;
color:black
}
Original file line number Diff line number Diff line change
@@ -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);
Expand Down
Original file line number Diff line number Diff line change
@@ -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>
Original file line number Diff line number Diff line change
@@ -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>
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import "./other.postcss";
export const indirect = "<dummy>";

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
/* not much to see */
Original file line number Diff line number Diff line change
@@ -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>
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Original file line number Diff line number Diff line change
@@ -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>
Original file line number Diff line number Diff line change
Expand Up @@ -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>
32 changes: 26 additions & 6 deletions packages/astro/e2e/view-transitions.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 }) => {
Expand Down Expand Up @@ -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');
Expand All @@ -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');
Expand Down
64 changes: 60 additions & 4 deletions packages/astro/src/transitions/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ const announce = () => {
};

const PERSIST_ATTR = 'data-astro-transition-persist';
const VITE_ID = 'data-vite-dev-id';

let parser: DOMParser;

Expand Down Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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();
}
Expand All @@ -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}"]`);
Expand All @@ -315,7 +319,6 @@ async function updateDOM(
newEl.replaceWith(el);
}
}

restoreFocus(savedFocus);

if (popState) {
Expand Down Expand Up @@ -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 }, '');
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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();
});
}
}
}
12 changes: 12 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 5c888c1

Please sign in to comment.