Skip to content

fix: make SvelteMap work with async mode#17908

Draft
Rich-Harris wants to merge 4 commits intoasync-svelte-setfrom
async-svelte-map
Draft

fix: make SvelteMap work with async mode#17908
Rich-Harris wants to merge 4 commits intoasync-svelte-setfrom
async-svelte-map

Conversation

@Rich-Harris
Copy link
Member

Pushing this up even though it has failing tests. The reason the new test fails is because we keep doing {await push(...)} every time a promise resolves, because the each block effect (which sets each item's .v source) re-runs on every traversal. This is because

  • we always add block effects to the #maybe_dirty_effects set on traversal
  • for some reason, is_dirty is returning true when it should be false

I intend to investigate both of these, because it seems like we're creating inefficiency at best, and incorrect behaviour at worst.

One thing I noticed is that over-execute deriveds when time-travelling, because effect_tracking() is false inside is_dirty:

// only cache the value if we're in a tracking context, otherwise we won't
// clear the cache in `mark_reactions` when dependencies are updated
if (effect_tracking() || current_batch?.is_fork) {
batch_values.set(derived, value);
}

If we could fix that, and get the invalidation right, then maybe we could preserve derived values across batch runs? e.g. we include them in batch.current, rather than just the ephemeral batch_values. Unfortunately I anticipate this being a deep rabbit hole

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

@changeset-bot
Copy link

changeset-bot bot commented Mar 12, 2026

⚠️ No Changeset found

Latest commit: 476f595

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

@Rich-Harris Rich-Harris changed the title Async svelte map fix: make SvelteSet work with async mode Mar 12, 2026
@Rich-Harris Rich-Harris changed the title fix: make SvelteSet work with async mode fix: make SvelteMap work with async mode Mar 12, 2026
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.

1 participant