Skip to content

fix: reject pending async deriveds on discard#18308

Merged
Rich-Harris merged 5 commits into
mainfrom
batch-discard-fix
Jun 1, 2026
Merged

fix: reject pending async deriveds on discard#18308
Rich-Harris merged 5 commits into
mainfrom
batch-discard-fix

Conversation

@dummdidumm

Copy link
Copy Markdown
Member

The rejects all async deriveds of a batch as OBSOLETE, so they don't hang around and bail early without triggering the batch.

If we don't do this, an async derived can trigger the already done batch, which schedules an effect that is never flushed. Because it is never flushed the branches it touched on its way up are never cleared, and so anything else in that subtree is now unreactive.

The rejects all async deriveds of a batch as `OBSOLETE`, so they don't hang around and bail early without triggering the batch.

If we don't do this, an async derived can trigger the already done batch, which schedules an effect that is never flushed. Because it is never flushed the branches it touched on its way up are never cleared, and so anything else in that subtree is now unreactive.
@changeset-bot

changeset-bot Bot commented May 27, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 61b8208

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

Copy link
Copy Markdown
Contributor

Playground

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

@svelte-docs-bot

Copy link
Copy Markdown

@Rich-Harris

Copy link
Copy Markdown
Member

looks like this causes a lot of legit test failures

@dummdidumm

Copy link
Copy Markdown
Member Author

This feels like another one of those "the fix is correct and revealed other bugs" cases. One of the failures (maybe all of them are similar) is around $effect.pending() creating a batch, and then wanting to discard the first batch (which doesn't have any writes, and therefore others.length is 0). That's obviously wrong.

@dummdidumm

Copy link
Copy Markdown
Member Author

Tests should all be passing now (if github CI ever runs). This PR revealed one bug and the fix needed one adjustment:

  • fix: our "this batch is obsolete" logic was wrong: certain batches (such as the very first one) are not triggered by a reactivity change, instead they are the root of a new tree. They have no sources (or only derived sources) in that case. We should never discard those.
  • adjustment: if you merge batch Y into X, and Y has a pending boundary, that means it has still-pending async work inside. To not discard those on commit of X we clear the map of async deriveds (we don't use them for anything else at this point so it's safe)

@Rich-Harris Rich-Harris merged commit a81f965 into main Jun 1, 2026
21 checks passed
@Rich-Harris Rich-Harris deleted the batch-discard-fix branch June 1, 2026 22:44
@github-actions github-actions Bot mentioned this pull request Jun 1, 2026
dummdidumm pushed a commit that referenced this pull request Jun 4, 2026
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and the packages will
be published to npm automatically. If you're not ready to do a release
yet, that's fine, whenever you add more changesets to main, this PR will
be updated.


# Releases
## svelte@5.56.2

### Patch Changes

- fix: properly track effect end node for async sibling component
([#18371](#18371))

- fix: prevent false-positive reactivity loss warning
([#18373](#18373))

- chore: bump esrap dependency
([#18372](#18372))

- fix: ignore declaration tags for animation directive
([#18366](#18366))

- fix: reject pending async deriveds on discard
([#18308](#18308))

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants