Skip to content

Conversation

dummdidumm
Copy link
Member

@dummdidumm dummdidumm commented Sep 3, 2025

Fixes #16627

This adjusts the logic of the first processing of an async boundary: Previously, it would only increment/decrement the pending count of a batch on subsequent runs and otherwise deactivate the batch, but this has multiple problems:

  • batches will get deactivated, and so certain changes (like chains of top level async deriveds) would create multiple batches, which is wrong. They should all be associated with the same batch
  • while async work is happening, the scheduled flush of the batch runs, and will run any $effects that were already picked up, because from its point of view there is no pending work

Additionally, currently after async work has finished which causes more effects (from within attachments, or top level $effects after async work) to be created this effects are never run because they are marked as dirty, but there's no flush scheduled.

The fix is

  • to always increment/decrement the pending count Update: Turns out this was on purpose, so we gotta keep not incrementing/decrementing on pending boundaries, else a state change that has causes a pending boundary to appear will be deferred which we don't want, since the async work happens "hidden" behind the pending boundary; added a test. We need to solve this differently by not collecting these effects until all async work is done
  • to run the flush work in a decrement in a microtask to give room for subsequent effects to be created and picked up (done, this fixes Attachment in Async / Await Component not working #16627)
  • to properly assign component $effects to the component context (todo)

This also cleans up our scheduling system - everything goes through queue_micro_task now, before this change batching was always in microtasks (meaning flushSync wouldn't really work correctly) and in some weird order (LIFO insteand of FIFO), and flushSync didn't take into account pending tasks.

There's some tests failing. One of them needs #16698 merged first, another one needs #16621 (that test was always failing when you tried it in the playground, weirdly the test passed up until now for some reason). The remaining need investigation.

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

Copy link

changeset-bot bot commented Sep 3, 2025

🦋 Changeset detected

Latest commit: 03f6287

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

@dummdidumm dummdidumm changed the title Boundary batch first run fix: correct first processing of boundary with pending snippet Sep 3, 2025
Copy link
Contributor

github-actions bot commented Sep 3, 2025

Playground

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

@svelte-docs-bot
Copy link


if (error) {
if (error !== STALE_REACTION) {
if (error !== STALE_REACTION && !batch.branch_obsolete(effect)) {
Copy link
Member Author

@dummdidumm dummdidumm Sep 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without this the enhanced async-block-reject-each-during-init test would fail, in other words this fixes a not-yet-reported bug

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.

Attachment in Async / Await Component not working
2 participants