Prevent rerunning scripts already ran in router (#12985)

* Prevent rerunning scripts already ran in router In a long session you might navigate between several pages, some contain the script and some not, but nevertheless the script should run a total of 1 time. This also helps with smaller bundled scripts in production, where some are inlined. * Add changeset * better key * more concise * review changes * move stuff around * fix types * take Martin's suggestion * run detection when executing scripts * adds an e2e test for data-astro-rerun * fix support for data-astro-rerun --------- Co-authored-by: Martin Trapp <[email protected]>
withastro · Mar 3, 2025 · 84e94cc · 84e94cc
1 parent fac32ad
commit 84e94cc
Show file tree

Hide file tree

Showing 9 changed files with 120 additions and 17 deletions.
diff --git a/.changeset/forty-houses-taste.md b/.changeset/forty-houses-taste.md
@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Prevent re-executing scripts in client router
diff --git a/packages/astro/e2e/fixtures/view-transitions/src/pages/eight.astro b/packages/astro/e2e/fixtures/view-transitions/src/pages/eight.astro
@@ -0,0 +1,16 @@
+---
+import Layout from '../components/Layout.astro';
+import { ClientRouter } from 'astro:transitions';
+
+Astro.response.headers.set('Content-Type', 'text/html ; charset=utf-8');
+---
+<html>
+	<head>
+		<ClientRouter handleForms />
+	</head>
+	<body>
+		<p id="eight">Page 8</p>
+
+		<a id="click-one" href="/one">go to 1</a>
+	</body>
+</html>
diff --git a/packages/astro/e2e/fixtures/view-transitions/src/pages/one.astro b/packages/astro/e2e/fixtures/view-transitions/src/pages/one.astro
@@ -7,6 +7,7 @@ import Layout from '../components/Layout.astro';
 	<a id="click-two" href="/two">go to 2</a>
 	<a id="click-three" href="/three">go to 3</a>
 	<a id="click-seven" href="/seven">go to 7</a>
+	<a id="click-eight" href="/eight">go to 8</a>
 	<a id="click-longpage" href="/long-page">go to long page</a>
 	<a id="click-self" href="">go to top</a>
 	<a id="click-redirect-two" href="/redirect-two">go to redirect 2</a>

diff --git a/packages/astro/e2e/fixtures/view-transitions/src/pages/page1-with-scripts.astro b/packages/astro/e2e/fixtures/view-transitions/src/pages/page1-with-scripts.astro
@@ -0,0 +1,15 @@
+---
+import {ClientRouter} from "astro:transitions";
+---
+<html>
+	<head>
+		<meta charset="utf-8" />
+		<ClientRouter />
+		<title>Page 1</title>
+		<script is:inline data-astro-rerun>console.log('[test] 3');</script>
+		<script is:inline>console.log('[test] 1');</script>
+	</head>
+	<body>
+		<a id="link" href="/page2-with-scripts">to page 2</a>
+	</body>
+</html>
diff --git a/packages/astro/e2e/fixtures/view-transitions/src/pages/page2-with-scripts.astro b/packages/astro/e2e/fixtures/view-transitions/src/pages/page2-with-scripts.astro
@@ -0,0 +1,15 @@
+---
+import {ClientRouter} from "astro:transitions";
+---
+<html>
+	<head>
+		<meta charset="utf-8" />
+		<ClientRouter />
+		<title>Page 2</title>
+		<script is:inline data-astro-rerun>console.log('[test] 2');</script>
+		<script is:inline>console.log('[test] 1');</script>
+	</head>
+	<body>
+		<a id="link" href="/page3-with-scripts">to page 3</a>
+	</body>
+</html>
diff --git a/packages/astro/e2e/fixtures/view-transitions/src/pages/page3-with-scripts.astro b/packages/astro/e2e/fixtures/view-transitions/src/pages/page3-with-scripts.astro
@@ -0,0 +1,16 @@
+---
+import {ClientRouter} from "astro:transitions";
+---
+<html>
+	<head>
+		<meta charset="utf-8" />
+		<ClientRouter />
+		<title>Page 3</title>
+		<script is:inline>console.log('[test] 1');</script>
+		<script is:inline data-astro-rerun>console.log('[test] 2');</script>
+		<script is:inline data-astro-rerun>console.log('[test] 3');</script>
+	</head>
+	<body>
+		<a id="link" href="/page1-with-scripts">to page 1</a>
+	</body>
+</html>
diff --git a/packages/astro/e2e/view-transitions.test.js b/packages/astro/e2e/view-transitions.test.js
@@ -647,14 +647,29 @@ test.describe('View Transitions', () => {
 	test('Scripts are only executed once', async ({ page, astro }) => {
 		// Go to page 1
 		await page.goto(astro.resolveUrl('/one'));
-		const p = page.locator('#one');
+		let p = page.locator('#one');
 		await expect(p, 'should have content').toHaveText('Page 1');
 
 		// go to page 2
 		await page.click('#click-two');
 		const article = page.locator('#twoarticle');
 		await expect(article, 'should have script content').toHaveText('works');
 
+		// Go back to page 1
+		await page.goBack();
+		p = page.locator('#one');
+		await expect(p, 'should have content').toHaveText('Page 1');
+
+		// Go to page 8
+		await page.click('#click-eight');
+		const article8 = page.locator('#eight');
+		await expect(article8, 'should have content').toHaveText('Page 8');
+
+		// Go back to page 1
+		await page.goBack();
+		p = page.locator('#one');
+		await expect(p, 'should have content').toHaveText('Page 1');
+
 		const meta = page.locator('[name="script-executions"]');
 		await expect(meta).toHaveAttribute('content', '0');
 	});
@@ -1588,4 +1603,20 @@ test.describe('View Transitions', () => {
 		await page.click('#click-two');
 		expect(lines.join('\n')).toBe(expected);
 	});
+
+	test('astro-data-rerun reruns known scripts', async ({ page, astro }) => {
+		let lines = [];
+		page.on('console', (msg) => {
+			msg.text().startsWith('[test]') && lines.push(msg.text().slice('[test]'.length + 1));
+		});
+		await page.goto(astro.resolveUrl('/page1-with-scripts'));
+		await expect(page).toHaveTitle('Page 1');
+		await page.click('#link');
+		await expect(page).toHaveTitle('Page 2');
+		await page.click('#link');
+		await expect(page).toHaveTitle('Page 3');
+		await page.click('#link');
+		await expect(page).toHaveTitle('Page 1');
+		expect(lines.join("")).toBe('312233');
+	});
 });
diff --git a/packages/astro/src/transitions/router.ts b/packages/astro/src/transitions/router.ts
@@ -1,6 +1,7 @@
 import type { TransitionBeforePreparationEvent } from './events.js';
 import { TRANSITION_AFTER_SWAP, doPreparation, doSwap } from './events.js';
 import type { Direction, Fallback, Options } from './types.js';
+import { detectScriptExecuted } from './swap-functions.js';
 
 type State = {
 	index: number;
@@ -646,7 +647,7 @@ if (inBrowser) {
 		}
 	}
 	for (const script of document.getElementsByTagName('script')) {
-		script.dataset.astroExec = '';
+		detectScriptExecuted(script);
 	}
 }
 

diff --git a/packages/astro/src/transitions/swap-functions.ts b/packages/astro/src/transitions/swap-functions.ts
@@ -6,25 +6,28 @@ export type SavedFocus = {
 
 const PERSIST_ATTR = 'data-astro-transition-persist';
 
+const scriptsAlreadyRan = new Set<string>();
+export function detectScriptExecuted(script: HTMLScriptElement) {
+	const key = script.src ? new URL(script.src, location.href).href : script.textContent!;
+	if (scriptsAlreadyRan.has(key)) return true;
+	scriptsAlreadyRan.add(key);
+	return false;
+}
+
 /*
  * 	Mark new scripts that should not execute
  */
 export function deselectScripts(doc: Document) {
-	for (const s1 of document.scripts) {
-		for (const s2 of doc.scripts) {
-			if (
-				// Check if the script should be rerun regardless of it being the same
-				!s2.hasAttribute('data-astro-rerun') &&
-				// Inline
-				((!s1.src && s1.textContent === s2.textContent) ||
-					// External
-					(s1.src && s1.type === s2.type && s1.src === s2.src))
-			) {
-				// the old script is in the new document and doesn't have the rerun attribute
-				// we mark it as executed to prevent re-execution
-				s2.dataset.astroExec = '';
-				break;
-			}
+	for (const s2 of doc.scripts) {
+		if (
+			// Check if the script should be rerun regardless of it being the same
+			!s2.hasAttribute('data-astro-rerun') &&
+			// Check if the script has already been executed
+			detectScriptExecuted(s2)
+		) {
+			// the old script is in the new document and doesn't have the rerun attribute
+			// we mark it as executed to prevent re-execution
+			s2.dataset.astroExec = '';
 		}
 	}
 }