Skip to content

Commit 84e94cc

Browse files
matthewpmartrapp
matthewp
and
martrapp
authored
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]>
1 parent fac32ad commit 84e94cc

File tree

9 files changed

+120
-17
lines changed

9 files changed

+120
-17
lines changed

‎.changeset/forty-houses-taste.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'astro': patch
3+
---
4+
5+
Prevent re-executing scripts in client router

‎packages/astro/e2e/fixtures/view-transitions/src/pages/eight.astro

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
---
2+
import Layout from '../components/Layout.astro';
3+
import { ClientRouter } from 'astro:transitions';
4+
5+
Astro.response.headers.set('Content-Type', 'text/html ; charset=utf-8');
6+
---
7+
<html>
8+
<head>
9+
<ClientRouter handleForms />
10+
</head>
11+
<body>
12+
<p id="eight">Page 8</p>
13+
14+
<a id="click-one" href="/one">go to 1</a>
15+
</body>
16+
</html>

‎packages/astro/e2e/fixtures/view-transitions/src/pages/one.astro

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import Layout from '../components/Layout.astro';
77
<a id="click-two" href="/two">go to 2</a>
88
<a id="click-three" href="/three">go to 3</a>
99
<a id="click-seven" href="/seven">go to 7</a>
10+
<a id="click-eight" href="/eight">go to 8</a>
1011
<a id="click-longpage" href="/long-page">go to long page</a>
1112
<a id="click-self" href="">go to top</a>
1213
<a id="click-redirect-two" href="/redirect-two">go to redirect 2</a>

‎packages/astro/e2e/fixtures/view-transitions/src/pages/page1-with-scripts.astro

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
---
2+
import {ClientRouter} from "astro:transitions";
3+
---
4+
<html>
5+
<head>
6+
<meta charset="utf-8" />
7+
<ClientRouter />
8+
<title>Page 1</title>
9+
<script is:inline data-astro-rerun>console.log('[test] 3');</script>
10+
<script is:inline>console.log('[test] 1');</script>
11+
</head>
12+
<body>
13+
<a id="link" href="/page2-with-scripts">to page 2</a>
14+
</body>
15+
</html>

‎packages/astro/e2e/fixtures/view-transitions/src/pages/page2-with-scripts.astro

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
---
2+
import {ClientRouter} from "astro:transitions";
3+
---
4+
<html>
5+
<head>
6+
<meta charset="utf-8" />
7+
<ClientRouter />
8+
<title>Page 2</title>
9+
<script is:inline data-astro-rerun>console.log('[test] 2');</script>
10+
<script is:inline>console.log('[test] 1');</script>
11+
</head>
12+
<body>
13+
<a id="link" href="/page3-with-scripts">to page 3</a>
14+
</body>
15+
</html>

‎packages/astro/e2e/fixtures/view-transitions/src/pages/page3-with-scripts.astro

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
---
2+
import {ClientRouter} from "astro:transitions";
3+
---
4+
<html>
5+
<head>
6+
<meta charset="utf-8" />
7+
<ClientRouter />
8+
<title>Page 3</title>
9+
<script is:inline>console.log('[test] 1');</script>
10+
<script is:inline data-astro-rerun>console.log('[test] 2');</script>
11+
<script is:inline data-astro-rerun>console.log('[test] 3');</script>
12+
</head>
13+
<body>
14+
<a id="link" href="/page1-with-scripts">to page 1</a>
15+
</body>
16+
</html>

‎packages/astro/e2e/view-transitions.test.js

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -647,14 +647,29 @@ test.describe('View Transitions', () => {
647647
test('Scripts are only executed once', async ({ page, astro }) => {
648648
// Go to page 1
649649
await page.goto(astro.resolveUrl('/one'));
650-
const p = page.locator('#one');
650+
let p = page.locator('#one');
651651
await expect(p, 'should have content').toHaveText('Page 1');
652652

653653
// go to page 2
654654
await page.click('#click-two');
655655
const article = page.locator('#twoarticle');
656656
await expect(article, 'should have script content').toHaveText('works');
657657

658+
// Go back to page 1
659+
await page.goBack();
660+
p = page.locator('#one');
661+
await expect(p, 'should have content').toHaveText('Page 1');
662+
663+
// Go to page 8
664+
await page.click('#click-eight');
665+
const article8 = page.locator('#eight');
666+
await expect(article8, 'should have content').toHaveText('Page 8');
667+
668+
// Go back to page 1
669+
await page.goBack();
670+
p = page.locator('#one');
671+
await expect(p, 'should have content').toHaveText('Page 1');
672+
658673
const meta = page.locator('[name="script-executions"]');
659674
await expect(meta).toHaveAttribute('content', '0');
660675
});
@@ -1588,4 +1603,20 @@ test.describe('View Transitions', () => {
15881603
await page.click('#click-two');
15891604
expect(lines.join('\n')).toBe(expected);
15901605
});
1606+
1607+
test('astro-data-rerun reruns known scripts', async ({ page, astro }) => {
1608+
let lines = [];
1609+
page.on('console', (msg) => {
1610+
msg.text().startsWith('[test]') && lines.push(msg.text().slice('[test]'.length + 1));
1611+
});
1612+
await page.goto(astro.resolveUrl('/page1-with-scripts'));
1613+
await expect(page).toHaveTitle('Page 1');
1614+
await page.click('#link');
1615+
await expect(page).toHaveTitle('Page 2');
1616+
await page.click('#link');
1617+
await expect(page).toHaveTitle('Page 3');
1618+
await page.click('#link');
1619+
await expect(page).toHaveTitle('Page 1');
1620+
expect(lines.join("")).toBe('312233');
1621+
});
15911622
});

‎packages/astro/src/transitions/router.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import type { TransitionBeforePreparationEvent } from './events.js';
22
import { TRANSITION_AFTER_SWAP, doPreparation, doSwap } from './events.js';
33
import type { Direction, Fallback, Options } from './types.js';
4+
import { detectScriptExecuted } from './swap-functions.js';
45

56
type State = {
67
index: number;
@@ -646,7 +647,7 @@ if (inBrowser) {
646647
}
647648
}
648649
for (const script of document.getElementsByTagName('script')) {
649-
script.dataset.astroExec = '';
650+
detectScriptExecuted(script);
650651
}
651652
}
652653

‎packages/astro/src/transitions/swap-functions.ts

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -6,25 +6,28 @@ export type SavedFocus = {
66

77
const PERSIST_ATTR = 'data-astro-transition-persist';
88

9+
const scriptsAlreadyRan = new Set<string>();
10+
export function detectScriptExecuted(script: HTMLScriptElement) {
11+
const key = script.src ? new URL(script.src, location.href).href : script.textContent!;
12+
if (scriptsAlreadyRan.has(key)) return true;
13+
scriptsAlreadyRan.add(key);
14+
return false;
15+
}
16+
917
/*
1018
* Mark new scripts that should not execute
1119
*/
1220
export function deselectScripts(doc: Document) {
13-
for (const s1 of document.scripts) {
14-
for (const s2 of doc.scripts) {
15-
if (
16-
// Check if the script should be rerun regardless of it being the same
17-
!s2.hasAttribute('data-astro-rerun') &&
18-
// Inline
19-
((!s1.src && s1.textContent === s2.textContent) ||
20-
// External
21-
(s1.src && s1.type === s2.type && s1.src === s2.src))
22-
) {
23-
// the old script is in the new document and doesn't have the rerun attribute
24-
// we mark it as executed to prevent re-execution
25-
s2.dataset.astroExec = '';
26-
break;
27-
}
21+
for (const s2 of doc.scripts) {
22+
if (
23+
// Check if the script should be rerun regardless of it being the same
24+
!s2.hasAttribute('data-astro-rerun') &&
25+
// Check if the script has already been executed
26+
detectScriptExecuted(s2)
27+
) {
28+
// the old script is in the new document and doesn't have the rerun attribute
29+
// we mark it as executed to prevent re-execution
30+
s2.dataset.astroExec = '';
2831
}
2932
}
3033
}

0 commit comments

Comments
 (0)