From a27f3e0973390b48bc6fa790171d3359a7d863bf Mon Sep 17 00:00:00 2001 From: Matthew Phillips Date: Tue, 19 Sep 2023 15:44:02 -0400 Subject: [PATCH 1/4] Prevent body scripts from re-executing on navigation --- .../astro/components/ViewTransitions.astro | 27 ++++++++++--------- .../src/components/InlineScript.astro | 9 +++++++ .../src/pages/inline-script-one.astro | 8 ++++++ .../src/pages/inline-script-two.astro | 8 ++++++ packages/astro/e2e/view-transitions.test.js | 18 +++++++++++++ 5 files changed, 57 insertions(+), 13 deletions(-) create mode 100644 packages/astro/e2e/fixtures/view-transitions/src/components/InlineScript.astro create mode 100644 packages/astro/e2e/fixtures/view-transitions/src/pages/inline-script-one.astro create mode 100644 packages/astro/e2e/fixtures/view-transitions/src/pages/inline-script-two.astro diff --git a/packages/astro/components/ViewTransitions.astro b/packages/astro/components/ViewTransitions.astro index 9c0a9dfdd9a6..7cd9ab110012 100644 --- a/packages/astro/components/ViewTransitions.astro +++ b/packages/astro/components/ViewTransitions.astro @@ -142,19 +142,6 @@ const { fallback = 'animate' } = Astro.props as Props; const href = el.getAttribute('href'); return doc.head.querySelector(`link[rel=stylesheet][href="${href}"]`); } - if (el.tagName === 'SCRIPT') { - let s1 = el as HTMLScriptElement; - for (const s2 of doc.scripts) { - if ( - // Inline - (s1.textContent && s1.textContent === s2.textContent) || - // External - (s1.type === s2.type && s1.src === s2.src) - ) { - return s2; - } - } - } // Only run this in dev. This will get stripped from production builds and is not needed. if (import.meta.env.DEV) { if (el.tagName === 'STYLE' && el.dataset.viteDevId) { @@ -202,6 +189,20 @@ const { fallback = 'animate' } = Astro.props as Props; // Everything left in the new head is new, append it all. document.head.append(...doc.head.children); + // Replace scripts + for(const s1 of document.scripts) { + for (const s2 of doc.scripts) { + if ( + // Inline + (s1.textContent && s1.textContent === s2.textContent) || + // External + (s1.type === s2.type && s1.src === s2.src) + ) { + s2.remove(); + } + } + } + // Persist elements in the existing body const oldBody = document.body; document.body.replaceWith(doc.body); diff --git a/packages/astro/e2e/fixtures/view-transitions/src/components/InlineScript.astro b/packages/astro/e2e/fixtures/view-transitions/src/components/InlineScript.astro new file mode 100644 index 000000000000..5418c5a64cd2 --- /dev/null +++ b/packages/astro/e2e/fixtures/view-transitions/src/components/InlineScript.astro @@ -0,0 +1,9 @@ +
Count
+ diff --git a/packages/astro/e2e/fixtures/view-transitions/src/pages/inline-script-one.astro b/packages/astro/e2e/fixtures/view-transitions/src/pages/inline-script-one.astro new file mode 100644 index 000000000000..e887fe6a5e77 --- /dev/null +++ b/packages/astro/e2e/fixtures/view-transitions/src/pages/inline-script-one.astro @@ -0,0 +1,8 @@ +--- +import Layout from '../components/Layout.astro'; +import InlineScript from '../components/InlineScript.astro'; +--- + + + Go to 2 + diff --git a/packages/astro/e2e/fixtures/view-transitions/src/pages/inline-script-two.astro b/packages/astro/e2e/fixtures/view-transitions/src/pages/inline-script-two.astro new file mode 100644 index 000000000000..430ad946517e --- /dev/null +++ b/packages/astro/e2e/fixtures/view-transitions/src/pages/inline-script-two.astro @@ -0,0 +1,8 @@ +--- +import Layout from '../components/Layout.astro'; +import InlineScript from '../components/InlineScript.astro'; +--- + + + Go to 1 + diff --git a/packages/astro/e2e/view-transitions.test.js b/packages/astro/e2e/view-transitions.test.js index 868660f9fb30..09f2c75143f1 100644 --- a/packages/astro/e2e/view-transitions.test.js +++ b/packages/astro/e2e/view-transitions.test.js @@ -663,4 +663,22 @@ test.describe('View Transitions', () => { locator = page.locator('#click-one'); await expect(locator).not.toBeInViewport(); }); + + test('body inline scripts do not re-execute on navigation', async ({ page, astro }) => { + const errors = []; + page.addListener('pageerror', err => { + errors.push(err); + }); + + await page.goto(astro.resolveUrl('/inline-script-one')); + let article = page.locator('#counter'); + await expect(article, 'should have script content').toBeVisible('exists'); + + await page.click('#click-one'); + + article = page.locator('#counter'); + await expect(article, 'should have script content').toHaveText('Count: 3'); + + expect(errors).toHaveLength(0); + }); }); From 09ac2522a175a7e684a2915a9f747d100245861a Mon Sep 17 00:00:00 2001 From: Matthew Phillips Date: Tue, 19 Sep 2023 15:44:54 -0400 Subject: [PATCH 2/4] Adding changeset --- .changeset/warm-turkeys-develop.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/warm-turkeys-develop.md diff --git a/.changeset/warm-turkeys-develop.md b/.changeset/warm-turkeys-develop.md new file mode 100644 index 000000000000..844232254174 --- /dev/null +++ b/.changeset/warm-turkeys-develop.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Prevent body scripts from re-executing on navigation From f8d8f84d32a705abcac0759f693d25d00f3ef713 Mon Sep 17 00:00:00 2001 From: Matthew Phillips Date: Tue, 19 Sep 2023 16:18:01 -0400 Subject: [PATCH 3/4] Run script replacement logic before head --- .../astro/components/ViewTransitions.astro | 31 ++++++++++--------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/packages/astro/components/ViewTransitions.astro b/packages/astro/components/ViewTransitions.astro index 7cd9ab110012..7ef84b55cb0c 100644 --- a/packages/astro/components/ViewTransitions.astro +++ b/packages/astro/components/ViewTransitions.astro @@ -174,6 +174,22 @@ const { fallback = 'animate' } = Astro.props as Props; html.setAttribute(name, value) ); + // Replace scripts in both the head and body. + for(const s1 of document.scripts) { + for (const s2 of doc.scripts) { + if ( + // Inline + (s1.textContent && s1.textContent === s2.textContent) || + // External + (s1.type === s2.type && s1.src === s2.src) + ) { + s2.remove(); + } else { + s1.remove(); + } + } + } + // Swap head for (const el of Array.from(document.head.children)) { const newEl = persistedHeadElement(el as HTMLElement); @@ -186,23 +202,10 @@ const { fallback = 'animate' } = Astro.props as Props; el.remove(); } } + // Everything left in the new head is new, append it all. document.head.append(...doc.head.children); - // Replace scripts - for(const s1 of document.scripts) { - for (const s2 of doc.scripts) { - if ( - // Inline - (s1.textContent && s1.textContent === s2.textContent) || - // External - (s1.type === s2.type && s1.src === s2.src) - ) { - s2.remove(); - } - } - } - // Persist elements in the existing body const oldBody = document.body; document.body.replaceWith(doc.body); From 3d8477e6b931a4d528d4713c98437046061d0c98 Mon Sep 17 00:00:00 2001 From: Matthew Phillips Date: Wed, 20 Sep 2023 08:36:01 -0400 Subject: [PATCH 4/4] Rename doc to newDocument --- .../astro/components/ViewTransitions.astro | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/packages/astro/components/ViewTransitions.astro b/packages/astro/components/ViewTransitions.astro index 7ef84b55cb0c..1e59d89d95b8 100644 --- a/packages/astro/components/ViewTransitions.astro +++ b/packages/astro/components/ViewTransitions.astro @@ -129,18 +129,18 @@ const { fallback = 'animate' } = Astro.props as Props; var noopEl = document.createElement('div'); } - async function updateDOM(doc: Document, loc: URL, state?: State, fallback?: Fallback) { + async function updateDOM(newDocument: Document, loc: URL, state?: State, fallback?: Fallback) { // Check for a head element that should persist, either because it has the data // attribute or is a link el. const persistedHeadElement = (el: HTMLElement): Element | null => { const id = el.getAttribute(PERSIST_ATTR); - const newEl = id && doc.head.querySelector(`[${PERSIST_ATTR}="${id}"]`); + const newEl = id && newDocument.head.querySelector(`[${PERSIST_ATTR}="${id}"]`); if (newEl) { return newEl; } if (el.matches('link[rel=stylesheet]')) { const href = el.getAttribute('href'); - return doc.head.querySelector(`link[rel=stylesheet][href="${href}"]`); + return newDocument.head.querySelector(`link[rel=stylesheet][href="${href}"]`); } // Only run this in dev. This will get stripped from production builds and is not needed. if (import.meta.env.DEV) { @@ -148,7 +148,7 @@ const { fallback = 'animate' } = Astro.props as Props; const devId = el.dataset.viteDevId; // If this same style tag exists, remove it from the new page return ( - doc.querySelector(`style[data-astro-dev-id="${devId}"]`) || + newDocument.querySelector(`style[data-astro-dev-id="${devId}"]`) || // Otherwise, keep it anyways. This is client:only styles. noopEl ); @@ -160,7 +160,7 @@ const { fallback = 'animate' } = Astro.props as Props; const swap = () => { // noscript tags inside head element are not honored on swap (#7969). // Remove them before swapping. - doc.querySelectorAll('head noscript').forEach((el) => el.remove()); + newDocument.querySelectorAll('head noscript').forEach((el) => el.remove()); // swap attributes of the html element // - delete all attributes from the current document @@ -170,13 +170,13 @@ const { fallback = 'animate' } = Astro.props as Props; const astro = [...html.attributes].filter( ({ name }) => (html.removeAttribute(name), name.startsWith('data-astro-')) ); - [...doc.documentElement.attributes, ...astro].forEach(({ name, value }) => + [...newDocument.documentElement.attributes, ...astro].forEach(({ name, value }) => html.setAttribute(name, value) ); // Replace scripts in both the head and body. for(const s1 of document.scripts) { - for (const s2 of doc.scripts) { + for (const s2 of newDocument.scripts) { if ( // Inline (s1.textContent && s1.textContent === s2.textContent) || @@ -204,11 +204,11 @@ const { fallback = 'animate' } = Astro.props as Props; } // Everything left in the new head is new, append it all. - document.head.append(...doc.head.children); + document.head.append(...newDocument.head.children); // Persist elements in the existing body const oldBody = document.body; - document.body.replaceWith(doc.body); + 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}"]`); @@ -251,7 +251,7 @@ const { fallback = 'animate' } = Astro.props as Props; // Wait on links to finish, to prevent FOUC const links: Promise[] = []; - for (const el of doc.querySelectorAll('head link[rel=stylesheet]')) { + for (const el of newDocument.querySelectorAll('head link[rel=stylesheet]')) { // Do not preload links that are already on the page. if ( !document.querySelector( @@ -303,8 +303,8 @@ const { fallback = 'animate' } = Astro.props as Props; return; } - const doc = parser.parseFromString(html, mediaType); - if (!doc.querySelector('[name="astro-view-transitions-enabled"]')) { + const newDocument = parser.parseFromString(html, mediaType); + if (!newDocument.querySelector('[name="astro-view-transitions-enabled"]')) { location.href = href; return; } @@ -314,9 +314,9 @@ const { fallback = 'animate' } = Astro.props as Props; document.documentElement.dataset.astroTransition = dir; if (supportsViewTransitions) { - finished = document.startViewTransition(() => updateDOM(doc, loc, state)).finished; + finished = document.startViewTransition(() => updateDOM(newDocument, loc, state)).finished; } else { - finished = updateDOM(doc, loc, state, getFallback()); + finished = updateDOM(newDocument, loc, state, getFallback()); } try { await finished;