fix: don't rerun async effects unnecessarily #16944

dummdidumm · 2025-10-14T12:08:41Z

Since #16866, when an async effect runs multiple times, we rebase older batches and rerun those effects. This can have unintended consequences: In a case where an async effect only depends on a single source, and that single source was updated in a later batch, we know that we don't need to / should not rerun the older batch.

This PR makes it so: We collect all the sources of older batches that are not part of the current batch that just committed, and then only mark those async effects as dirty which depend on one of those other sources. Fixes the bug I noticed while working on #16935

Before submitting the PR, please make sure you do the following

It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
Prefix your PR title with feat:, fix:, chore:, or docs:.
This message body should clearly illustrate what problems it solves.
Ideally, include a test that fails without this PR but passes with it.
If this PR changes code within packages/svelte/src, add a changeset (npx changeset).

Tests and linting

Run the tests with pnpm test and lint the project with pnpm lint

Since #16866, when an async effect runs multiple times, we rebase older batches and rerun those effects. This can have unintended consequences: In a case where an async effect only depends on a single source, and that single source was updated in a later batch, we know that we don't need to / should not rerun the older batch. This PR makes it so: We collect all the sources of older batches that are not part of the current batch that just committed, and then only mark those async effects as dirty which depend on one of those other sources. Fixes the bug I noticed while working on #16935

changeset-bot · 2025-10-14T12:08:45Z

🦋 Changeset detected

Latest commit: 697445b

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

This PR includes changesets to release 1 package

Name	Type
svelte	Patch

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

github-actions · 2025-10-14T12:09:33Z

Playground

pnpm add https://pkg.pr.new/svelte@16944

packages/svelte/src/internal/client/reactivity/batch.js

Rich-Harris

a few small comments but this LGTM, great work

Co-authored-by: Rich Harris <rich.harris@vercel.com>

quick follow-up to #16944 Resetting a map entry does not change its position in the map when iterating. We need to make sure that reset makes that batch jump "to the front" for the "reject all stale batches" logic below. Edge case for which I can't come up with a test case but it _is_ a possibility.

* runtime-first approach * revert these * type safety, lint * fix: better input cursor restoration for `bind:value` (#16925) If cursor was at end and new input is longer, move cursor to new end No test because not possible to reproduce using our test setup. Follow-up to #14649, helps with #16577 * Version Packages (#16920) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * docs: await no longer need pending (#16900) * docs: link to custom renderer issue in Svelte Native discussion (#16896) * fix code block (#16937) Updated code block syntax from Svelte to JavaScript for clarity. * fix: unset context on stale promises (#16935) * fix: unset context on stale promises When a stale promise is rejected in `async_derived`, and the promise eventually resolves, `d.resolve` will be noop and `d.promise.then(handler, ...)` will never run. That in turns means any restored context (via `(await save(..))()`) will never be unset. We have to handle this case and unset the context to prevent errors such as false-positive state mutation errors * fix: unset context on stale promises (slightly different approach) (#16936) * slightly different approach to #16935 * move unset_context call * get rid of logs --------- Co-authored-by: Rich Harris <rich.harris@vercel.com> * fix: svg `radialGradient` `fr` attribute missing in types (#16943) * fix(svg radialGradient): fr attribute missing in types * chore: add changeset * Version Packages (#16940) * Version Packages * Update packages/svelte/CHANGELOG.md --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Rich Harris <rich.harris@vercel.com> * chore: simplify `batch.apply()` (#16945) * chore: simplify `batch.apply()` * belt and braces * note to self * unused * fix: don't rerun async effects unnecessarily (#16944) Since #16866, when an async effect runs multiple times, we rebase older batches and rerun those effects. This can have unintended consequences: In a case where an async effect only depends on a single source, and that single source was updated in a later batch, we know that we don't need to / should not rerun the older batch. This PR makes it so: We collect all the sources of older batches that are not part of the current batch that just committed, and then only mark those async effects as dirty which depend on one of those other sources. Fixes the bug I noticed while working on #16935 * fix: ensure map iteration order is correct (#16947) quick follow-up to #16944 Resetting a map entry does not change its position in the map when iterating. We need to make sure that reset makes that batch jump "to the front" for the "reject all stale batches" logic below. Edge case for which I can't come up with a test case but it _is_ a possibility. * feat: add `createContext` utility for type-safe context (#16948) * feat: add `createContext` utility for type-safe context * regenerate * Version Packages (#16946) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * chore: Remove annoying sync-async warning (#16949) * fix * use `$state.eager(value)` instead of `$effect.pending(value)` --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Hyunbin Seo <47051820+hyunbinseo@users.noreply.github.com> Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com> Co-authored-by: Rich Harris <rich.harris@vercel.com> Co-authored-by: Hannes Rüger <hannes@hannesrueger.de> Co-authored-by: Elliott Johnson <sejohnson@torchcloudconsulting.com>

* WIP implement `$effect.pending(...)` * feat: `$state.eager(value)` (#16926) * runtime-first approach * revert these * type safety, lint * fix: better input cursor restoration for `bind:value` (#16925) If cursor was at end and new input is longer, move cursor to new end No test because not possible to reproduce using our test setup. Follow-up to #14649, helps with #16577 * Version Packages (#16920) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * docs: await no longer need pending (#16900) * docs: link to custom renderer issue in Svelte Native discussion (#16896) * fix code block (#16937) Updated code block syntax from Svelte to JavaScript for clarity. * fix: unset context on stale promises (#16935) * fix: unset context on stale promises When a stale promise is rejected in `async_derived`, and the promise eventually resolves, `d.resolve` will be noop and `d.promise.then(handler, ...)` will never run. That in turns means any restored context (via `(await save(..))()`) will never be unset. We have to handle this case and unset the context to prevent errors such as false-positive state mutation errors * fix: unset context on stale promises (slightly different approach) (#16936) * slightly different approach to #16935 * move unset_context call * get rid of logs --------- Co-authored-by: Rich Harris <rich.harris@vercel.com> * fix: svg `radialGradient` `fr` attribute missing in types (#16943) * fix(svg radialGradient): fr attribute missing in types * chore: add changeset * Version Packages (#16940) * Version Packages * Update packages/svelte/CHANGELOG.md --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Rich Harris <rich.harris@vercel.com> * chore: simplify `batch.apply()` (#16945) * chore: simplify `batch.apply()` * belt and braces * note to self * unused * fix: don't rerun async effects unnecessarily (#16944) Since #16866, when an async effect runs multiple times, we rebase older batches and rerun those effects. This can have unintended consequences: In a case where an async effect only depends on a single source, and that single source was updated in a later batch, we know that we don't need to / should not rerun the older batch. This PR makes it so: We collect all the sources of older batches that are not part of the current batch that just committed, and then only mark those async effects as dirty which depend on one of those other sources. Fixes the bug I noticed while working on #16935 * fix: ensure map iteration order is correct (#16947) quick follow-up to #16944 Resetting a map entry does not change its position in the map when iterating. We need to make sure that reset makes that batch jump "to the front" for the "reject all stale batches" logic below. Edge case for which I can't come up with a test case but it _is_ a possibility. * feat: add `createContext` utility for type-safe context (#16948) * feat: add `createContext` utility for type-safe context * regenerate * Version Packages (#16946) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * chore: Remove annoying sync-async warning (#16949) * fix * use `$state.eager(value)` instead of `$effect.pending(value)` --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Hyunbin Seo <47051820+hyunbinseo@users.noreply.github.com> Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com> Co-authored-by: Rich Harris <rich.harris@vercel.com> Co-authored-by: Hannes Rüger <hannes@hannesrueger.de> Co-authored-by: Elliott Johnson <sejohnson@torchcloudconsulting.com> * decouple from boundaries * use queue_micro_task * add test * fix * changeset * revert * tidy up * update docs * Update packages/svelte/src/internal/client/reactivity/batch.js Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com> * minor tweak --------- Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Hyunbin Seo <47051820+hyunbinseo@users.noreply.github.com> Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com> Co-authored-by: Hannes Rüger <hannes@hannesrueger.de> Co-authored-by: Elliott Johnson <sejohnson@torchcloudconsulting.com>

dummdidumm mentioned this pull request Oct 14, 2025

feat: $state.eager(value) (runtime-first approach) #16926

Merged

Rich-Harris reviewed Oct 14, 2025

View reviewed changes

packages/svelte/src/internal/client/reactivity/batch.js Show resolved Hide resolved

yoink out depends_on_sources

12b8ee6

Rich-Harris reviewed Oct 14, 2025

View reviewed changes

packages/svelte/src/internal/client/reactivity/batch.js Outdated Show resolved Hide resolved

Rich-Harris reviewed Oct 14, 2025

View reviewed changes

packages/svelte/src/internal/client/reactivity/batch.js Outdated Show resolved Hide resolved

Rich-Harris approved these changes Oct 14, 2025

View reviewed changes

Rich-Harris and others added 4 commits October 14, 2025 11:15

merge main

518fc79

Apply suggestion from @Rich-Harris

cffac17

Co-authored-by: Rich Harris <rich.harris@vercel.com>

Apply suggestion from @Rich-Harris

625d8e0

Co-authored-by: Rich Harris <rich.harris@vercel.com>

Update packages/svelte/src/internal/client/reactivity/batch.js

697445b

Co-authored-by: Rich Harris <rich.harris@vercel.com>

dummdidumm merged commit 28765f8 into main Oct 14, 2025
15 of 16 checks passed

dummdidumm deleted the promise-stale-fix-pt-2 branch October 14, 2025 15:59

github-actions bot mentioned this pull request Oct 14, 2025

Version Packages #16946

Merged

dummdidumm mentioned this pull request Oct 14, 2025

fix: ensure map iteration order is correct #16947

Merged

dummdidumm mentioned this pull request Oct 17, 2025

fix: cleanup old batches, run their still-current commits #16621

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix: don't rerun async effects unnecessarily #16944

fix: don't rerun async effects unnecessarily #16944

dummdidumm commented Oct 14, 2025

Uh oh!

changeset-bot bot commented Oct 14, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Oct 14, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Rich-Harris left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

fix: don't rerun async effects unnecessarily #16944

fix: don't rerun async effects unnecessarily #16944

Conversation

dummdidumm commented Oct 14, 2025

Before submitting the PR, please make sure you do the following

Tests and linting

Uh oh!

changeset-bot bot commented Oct 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

github-actions bot commented Oct 14, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Rich-Harris left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

changeset-bot bot commented Oct 14, 2025 •

edited

Loading