Prevent rerunning scripts already ran in router #12985

matthewp · 2025-01-14T20:44:40Z

Changes

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.
Keeps track of scripts that have run, either by the src or by the inner text (for inline scripts) and uses that as a key, rather than the data attribute we previously adde.

Testing

Updated the existing tests to navigate between more pages.

Docs

N/A, bug fix.

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.

changeset-bot · 2025-01-14T20:44:45Z

🦋 Changeset detected

Latest commit: aa4f9b8

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

codspeed-hq · 2025-01-14T20:57:44Z

CodSpeed Performance Report

Merging #12985 will not alter performance

_{Comparing rem-router-page (aa4f9b8) with main (3b66955)}

Summary

✅ 6 untouched benchmarks

martrapp

Looks like what we discussed on discord ;-)
Left two comments.

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

packages/astro/src/transitions/router.ts

martrapp · 2025-01-15T10:53:50Z

With that change in place, we can also remove

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

Lines 9 to 30 in 0968069

    
           /* 
        
            * 	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; 
        
           			} 
        
           		} 
        
           	} 
        
           }

and

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

Line 153 in 0968069

deselectScripts(doc);

matthewp · 2025-01-29T13:32:02Z

@martrapp deselectScripts is part of the public API, so we can't remove it. We can probably make it a noop though. Would that work?

matthewp · 2025-01-29T13:53:30Z

!preview router-noexec

github-actions · 2025-01-29T13:57:55Z

Snapshots have been released for the following packages:

@astrojs/markdoc@experimental--router-noexec
@astrojs/markdown-remark@experimental--router-noexec
astro@experimental--router-noexec
@astrojs/preact@experimental--router-noexec
@astrojs/react@experimental--router-noexec
@astrojs/solid-js@experimental--router-noexec
@astrojs/vue@experimental--router-noexec
@astrojs/mdx@experimental--router-noexec

Publish Log

🦋  warn ===============================IMPORTANT!===============================
🦋  warn Packages will be released under the experimental--router-noexec tag
🦋  warn ----------------------------------------------------------------------
🦋  info npm info astro
🦋  info npm info @astrojs/prism
🦋  info npm info @astrojs/rss
🦋  info npm info create-astro
🦋  info npm info @astrojs/db
🦋  info npm info @astrojs/alpinejs
🦋  info npm info @astrojs/markdoc
🦋  info npm info @astrojs/mdx
🦋  info npm info @astrojs/partytown
🦋  info npm info @astrojs/preact
🦋  info npm info @astrojs/react
🦋  info npm info @astrojs/sitemap
🦋  info npm info @astrojs/solid-js
🦋  info npm info @astrojs/svelte
🦋  info npm info @astrojs/tailwind
🦋  info npm info @astrojs/vue
🦋  info npm info @astrojs/web-vitals
🦋  info npm info @astrojs/internal-helpers
🦋  info npm info @astrojs/markdown-remark
🦋  info npm info @astrojs/studio
🦋  info npm info @astrojs/telemetry
🦋  info npm info @astrojs/underscore-redirects
🦋  info npm info @astrojs/upgrade
🦋  info astro is being published because our local version (0.0.0-router-noexec-20250129135648) has not been published on npm
🦋  warn @astrojs/prism is not being published because version 3.2.0 is already published on npm
🦋  warn @astrojs/rss is not being published because version 4.0.11 is already published on npm
🦋  warn create-astro is not being published because version 4.11.0 is already published on npm
🦋  warn @astrojs/db is not being published because version 0.14.5 is already published on npm
🦋  warn @astrojs/alpinejs is not being published because version 0.4.1 is already published on npm
🦋  info @astrojs/markdoc is being published because our local version (0.0.0-router-noexec-20250129135648) has not been published on npm
🦋  info @astrojs/mdx is being published because our local version (0.0.0-router-noexec-20250129135648) has not been published on npm
🦋  warn @astrojs/partytown is not being published because version 2.1.3 is already published on npm
🦋  info @astrojs/preact is being published because our local version (0.0.0-router-noexec-20250129135648) has not been published on npm
🦋  info @astrojs/react is being published because our local version (0.0.0-router-noexec-20250129135648) has not been published on npm
🦋  warn @astrojs/sitemap is not being published because version 3.2.1 is already published on npm
🦋  info @astrojs/solid-js is being published because our local version (0.0.0-router-noexec-20250129135648) has not been published on npm
🦋  warn @astrojs/svelte is not being published because version 7.0.3 is already published on npm
🦋  warn @astrojs/tailwind is not being published because version 5.1.4 is already published on npm
🦋  info @astrojs/vue is being published because our local version (0.0.0-router-noexec-20250129135648) has not been published on npm
🦋  warn @astrojs/web-vitals is not being published because version 3.0.1 is already published on npm
🦋  warn @astrojs/internal-helpers is not being published because version 0.4.2 is already published on npm
🦋  info @astrojs/markdown-remark is being published because our local version (0.0.0-router-noexec-20250129135648) has not been published on npm
🦋  warn @astrojs/studio is not being published because version 0.1.3 is already published on npm
🦋  warn @astrojs/telemetry is not being published because version 3.2.0 is already published on npm
🦋  warn @astrojs/underscore-redirects is not being published because version 0.6.0 is already published on npm
🦋  warn @astrojs/upgrade is not being published because version 0.4.3 is already published on npm
🦋  info Publishing "astro" at "0.0.0-router-noexec-20250129135648"
🦋  info Publishing "@astrojs/markdoc" at "0.0.0-router-noexec-20250129135648"
🦋  info Publishing "@astrojs/mdx" at "0.0.0-router-noexec-20250129135648"
🦋  info Publishing "@astrojs/preact" at "0.0.0-router-noexec-20250129135648"
🦋  info Publishing "@astrojs/react" at "0.0.0-router-noexec-20250129135648"
🦋  info Publishing "@astrojs/solid-js" at "0.0.0-router-noexec-20250129135648"
🦋  info Publishing "@astrojs/vue" at "0.0.0-router-noexec-20250129135648"
🦋  info Publishing "@astrojs/markdown-remark" at "0.0.0-router-noexec-20250129135648"
🦋  success packages published successfully:
🦋  [email protected]
🦋  @astrojs/[email protected]
🦋  @astrojs/[email protected]
🦋  @astrojs/[email protected]
🦋  @astrojs/[email protected]
🦋  @astrojs/[email protected]
🦋  @astrojs/[email protected]
🦋  @astrojs/[email protected]
🦋  Creating git tags...
🦋  New tag:  [email protected]
🦋  New tag:  @astrojs/[email protected]
🦋  New tag:  @astrojs/[email protected]
🦋  New tag:  @astrojs/[email protected]
🦋  New tag:  @astrojs/[email protected]
🦋  New tag:  @astrojs/[email protected]
🦋  New tag:  @astrojs/[email protected]
🦋  New tag:  @astrojs/[email protected]

Build Log


> [email protected] build /home/runner/work/astro/astro
> turbo run build --filter=astro --filter=create-astro --filter="@astrojs/*" --filter="@benchmark/*"

• Packages in scope: @astrojs/alpinejs, @astrojs/cloudflare, @astrojs/db, @astrojs/internal-helpers, @astrojs/markdoc, @astrojs/markdown-remark, @astrojs/mdx, @astrojs/netlify, @astrojs/node, @astrojs/partytown, @astrojs/preact, @astrojs/prism, @astrojs/react, @astrojs/rss, @astrojs/sitemap, @astrojs/solid-js, @astrojs/studio, @astrojs/svelte, @astrojs/tailwind, @astrojs/telemetry, @astrojs/underscore-redirects, @astrojs/upgrade, @astrojs/vercel, @astrojs/vue, @astrojs/web-vitals, @benchmark/adapter, @benchmark/timer, astro, create-astro
• Running build in 29 packages
• Remote caching enabled
::group::@astrojs/prism:build
cache miss, executing d2e4f1a807203696

> @astrojs/[email protected] build /home/runner/work/astro/astro/packages/astro-prism
> astro-scripts build "src/**/*.ts" && tsc -p ./tsconfig.json

::endgroup::
::group::@astrojs/internal-helpers:build
cache miss, executing 603e13eb1d59a2fa

> @astrojs/[email protected] build /home/runner/work/astro/astro/packages/internal-helpers
> astro-scripts build "src/**/*.ts" && tsc -p tsconfig.json

::endgroup::
::group::@astrojs/telemetry:build
cache miss, executing 10fa8f920a868762

> @astrojs/[email protected] build /home/runner/work/astro/astro/packages/telemetry
> astro-scripts build "src/**/*.ts" && tsc

::endgroup::
::group::create-astro:build
cache miss, executing c7b2023fd2304e72

> [email protected] build /home/runner/work/astro/astro/packages/create-astro
> astro-scripts build "src/index.ts" --bundle && tsc

::endgroup::
::group::@astrojs/upgrade:build
cache miss, executing c0d7b7528eded078

> @astrojs/[email protected] build /home/runner/work/astro/astro/packages/upgrade
> astro-scripts build "src/index.ts" --bundle && tsc

::endgroup::
::group::@astrojs/markdown-remark:build
cache miss, executing 54013287f1f88f88

> @astrojs/[email protected] build /home/runner/work/astro/astro/packages/markdown/remark
> astro-scripts build "src/**/*.ts" && tsc -p tsconfig.json

::endgroup::
::group::astro:build
cache miss, executing 515928385e8f6ca0

> [email protected] build /home/runner/work/astro/astro/packages/astro
> pnpm run prebuild && astro-scripts build "src/**/*.{ts,js}" --copy-wasm && tsc


> [email protected] prebuild /home/runner/work/astro/astro/packages/astro
> astro-scripts prebuild --to-string "src/runtime/server/astro-island.ts" "src/runtime/client/{idle,load,media,only,visible}.ts"

::endgroup::
::group::@astrojs/rss:build
cache miss, executing 554e1691fc2c0fa0

> @astrojs/[email protected] build /home/runner/work/astro/astro/packages/astro-rss
> astro-scripts build "src/**/*.ts" && tsc

::endgroup::
::group::@astrojs/partytown:build
cache miss, executing 02794d8d748acd4a

> @astrojs/[email protected] build /home/runner/work/astro/astro/packages/integrations/partytown
> astro-scripts build "src/**/*.ts" && tsc

::endgroup::
::group::@benchmark/timer:build
cache miss, executing e1cb1b0ae653899b

> @benchmark/[email protected] build /home/runner/work/astro/astro/benchmark/packages/timer
> astro-scripts build "src/**/*.ts" && tsc

::endgroup::
::group::@astrojs/alpinejs:build
cache miss, executing 258e044251c1a414

> @astrojs/[email protected] build /home/runner/work/astro/astro/packages/integrations/alpinejs
> astro-scripts build "src/**/*.ts" && tsc

::endgroup::
::group::@astrojs/svelte:build
cache miss, executing 5d051c2889876b2c

> @astrojs/[email protected] build /home/runner/work/astro/astro/packages/integrations/svelte
> astro-scripts build "src/index.ts" && astro-scripts build "src/editor.cts" --force-cjs --no-clean-dist && tsc

::endgroup::
::group::@astrojs/react:build
cache miss, executing 8da3937ecaf68433

> @astrojs/[email protected] build /home/runner/work/astro/astro/packages/integrations/react
> astro-scripts build "src/**/*.ts" && tsc

::endgroup::
::group::@astrojs/tailwind:build
cache miss, executing 8c33006b9dfbddbf

> @astrojs/[email protected] build /home/runner/work/astro/astro/packages/integrations/tailwind
> astro-scripts build "src/**/*.ts" && tsc

::endgroup::
::group::@astrojs/sitemap:build
cache miss, executing 7225d4386e55ae65

> @astrojs/[email protected] build /home/runner/work/astro/astro/packages/integrations/sitemap
> astro-scripts build "src/**/*.ts" && tsc

::endgroup::
::group::@astrojs/solid-js:build
cache miss, executing 8d8c421c695646c5

> @astrojs/[email protected] build /home/runner/work/astro/astro/packages/integrations/solid
> astro-scripts build "src/**/*.ts" && tsc

::endgroup::
::group::@astrojs/preact:build
cache miss, executing eea090d51bdd604d

> @astrojs/[email protected] build /home/runner/work/astro/astro/packages/integrations/preact
> astro-scripts build "src/**/*.ts" && tsc

::endgroup::
::group::@astrojs/studio:build
cache miss, executing 2245e52d258c47d9

> @astrojs/[email protected] build /home/runner/work/astro/astro/packages/studio
> astro-scripts build "src/**/*.ts" && tsc

::endgroup::
::group::@astrojs/underscore-redirects:build
cache miss, executing 7b1ba440c089ee90

> @astrojs/[email protected] build /home/runner/work/astro/astro/packages/underscore-redirects
> astro-scripts build "src/**/*.ts" && tsc -p tsconfig.json

::endgroup::
::group::@benchmark/adapter:build
cache miss, executing 03e37c8773187cd8

> @benchmark/[email protected] build /home/runner/work/astro/astro/benchmark/packages/adapter
> astro-scripts build "src/**/*.ts" && tsc

::endgroup::
::group::@astrojs/markdoc:build
cache miss, executing 6b906a6847a4068b

> @astrojs/[email protected] build /home/runner/work/astro/astro/packages/integrations/markdoc
> astro-scripts build "src/**/*.ts" && tsc

::endgroup::
::group::@astrojs/mdx:build
cache miss, executing e57eda0ff45f9cfd

> @astrojs/[email protected] build /home/runner/work/astro/astro/packages/integrations/mdx
> astro-scripts build "src/**/*.ts" && tsc

::endgroup::
::group::@astrojs/vue:build
cache miss, executing 32123ab768c87b6a

> @astrojs/[email protected] build /home/runner/work/astro/astro/packages/integrations/vue
> astro-scripts build "src/index.ts" && astro-scripts build "src/editor.cts" --force-cjs --no-clean-dist && tsc

::endgroup::
::group::@astrojs/db:build
cache miss, executing c97c8f394f049817

> @astrojs/[email protected] build /home/runner/work/astro/astro/packages/db
> astro-scripts build "src/**/*.ts" && tsc && pnpm types:virtual


> @astrojs/[email protected] types:virtual /home/runner/work/astro/astro/packages/db
> tsc -p ./tsconfig.virtual.json

::endgroup::
::group::@astrojs/web-vitals:build
cache miss, executing 529d11cf4b62d505

> @astrojs/[email protected] build /home/runner/work/astro/astro/packages/integrations/web-vitals
> astro-scripts build "src/**/*.ts" && tsc

::endgroup::

 Tasks:    25 successful, 25 total
Cached:    0 cached, 25 total
  Time:    47.086s

martrapp · 2025-01-29T14:40:54Z

deselectScripts is part of the public API, so we can't remove it. We can probably make it a noop though. Would that work?

Ah, right. The job of deselectScripts is to mark those scripts in the new DOM that should not execute. So it would be better to leave runScripts as it was and move the new detectScriptExecuted to deselectScripts.

matthewp · 2025-01-29T15:02:30Z

@martrapp I created a preview release, can you let the people who encountered the bug to try it out?

martrapp · 2025-01-29T15:12:19Z

@martrapp I created a preview release, can you let the people who encountered the bug to try it out?

Yes will do! Thanks a lot!

Btw: taking astroExec out of the loop also made data-astro-rerun ineffective, see deselectscripts

matthewp · 2025-01-31T14:37:16Z

We got 1 confirmation that it fixes their issue. Enough to merge this?

martrapp · 2025-01-31T15:05:14Z

Hi @matthewp,
the code as it is now will break data-astro-rerun.
It would be better to move the detectScriptExecuted logic to the deselectScripts function.

matthewp · 2025-02-05T14:00:39Z

Looks like that actually broke tests :(

martrapp · 2025-02-05T14:43:28Z

Yes, the code that checks for same src resp. same text re-establishes the previous semantics.
Remove the s1 loop and these lines

 &&
				// Inline
				((!s1.src && s1.textContent === s2.textContent) ||
					// External
					(s1.src && s1.type === s2.type && s1.src === s2.src))

and all should be fine.

github-actions bot added the pkg: astro Related to the core `astro` package (scope) label Jan 14, 2025

matthewp added 2 commits January 14, 2025 15:45

Add changeset

cd80fe6

better key

0686646

matthewp requested a review from martrapp January 14, 2025 20:45

more concise

fe845fe

martrapp reviewed Jan 15, 2025

View reviewed changes

packages/astro/e2e/view-transitions.test.js Show resolved Hide resolved

packages/astro/src/transitions/router.ts Outdated Show resolved Hide resolved

review changes

2ca1f97

This was referenced Jan 29, 2025

Astro v5 ClientRouter duplicating scripts when deployed #12804

Open

astro:page-load broken and inconsistent, breaks scripts #12858

Open

matthewp added 2 commits February 5, 2025 08:27

move stuff around

a06ca07

fix types

c79567d

take Martin's suggestion

aa4f9b8

ascorbic assigned matthewp Feb 12, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prevent rerunning scripts already ran in router #12985

Prevent rerunning scripts already ran in router #12985

matthewp commented Jan 14, 2025

changeset-bot bot commented Jan 14, 2025 •

edited

Loading

codspeed-hq bot commented Jan 14, 2025 •

edited

Loading

martrapp left a comment

martrapp commented Jan 15, 2025

matthewp commented Jan 29, 2025

matthewp commented Jan 29, 2025

github-actions bot commented Jan 29, 2025

martrapp commented Jan 29, 2025

matthewp commented Jan 29, 2025

martrapp commented Jan 29, 2025

matthewp commented Jan 31, 2025

martrapp commented Jan 31, 2025

matthewp commented Feb 5, 2025

martrapp commented Feb 5, 2025 •

edited

Loading

Prevent rerunning scripts already ran in router #12985

Are you sure you want to change the base?

Prevent rerunning scripts already ran in router #12985

Conversation

matthewp commented Jan 14, 2025

Changes

Testing

Docs

changeset-bot bot commented Jan 14, 2025 • edited Loading

🦋 Changeset detected

codspeed-hq bot commented Jan 14, 2025 • edited Loading

CodSpeed Performance Report

Merging #12985 will not alter performance

Summary

martrapp left a comment

Choose a reason for hiding this comment

martrapp commented Jan 15, 2025

matthewp commented Jan 29, 2025

matthewp commented Jan 29, 2025

github-actions bot commented Jan 29, 2025

martrapp commented Jan 29, 2025

matthewp commented Jan 29, 2025

martrapp commented Jan 29, 2025

matthewp commented Jan 31, 2025

martrapp commented Jan 31, 2025

matthewp commented Feb 5, 2025

martrapp commented Feb 5, 2025 • edited Loading

changeset-bot bot commented Jan 14, 2025 •

edited

Loading

codspeed-hq bot commented Jan 14, 2025 •

edited

Loading

martrapp commented Feb 5, 2025 •

edited

Loading