Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

fix: robustify async each blocks#17126

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Draft
dummdidumm wants to merge3 commits intomain
base:main
Choose a base branch
Loading
fromasync-each-fix
Draft

Conversation

@dummdidumm
Copy link
Member

@dummdidummdummdidumm commentedNov 7, 2025
edited
Loading

Fixes#17033
Fixes#17050

This one's hell, and has revelead three different bugs:

Bug 1

commit in an each could happen long after the original context of the block effect is gone. That meansget_boundary() inrun might not get the correct active effect, maybe it's evennull at that moment. To fix it Ithink we need to capture then restore the context. Loads of tests failed with that approach, so I added another capture-restore within the commit function itself but there's still one test failing - still trying to find out why.

Bug 2

Each blocks get out of order because of race conditions and block effects not rescheduling when they should. Reproducible by clicking fast three times on the button in the reproduction of#17033. This is what happens:

  1. run batch 1
  2. run batch 2
  3. run batch
  4. batch 1 completes, wants to rebase batch 2/3.
    • Does update and run batch 2 first.
      That reruns each block because each blocks rely on the newer values (because of proxy each entry is a separate source).
      Since each'sarray method is not time traveling it's always the value of whatever batch ran last. Rerun also causes reinit of Circle.svelte because each logic destroys the old one
    • Does NOT run batch 3 because no overlap according to our "depends on distinct values" logic (since each block just reran and now has more dependencies on it so false negative).
  5. batch 3 Circle.run completes, decrement 0, runs commit
    • Does NOT rebase batch 2 because of same reasons as 4.
  6. batch 2 Circle.run completes, decrement 0, runs commit -> wrong end result

The reason for this is that block effects can change dependencies after a rerun and so a subsequent batch rebase could have a false positive or negative effect reschedule. To fix it we need to split up the rebase logic into two stages: First collect all effects of all batches that should rerun, then run them in a second loop.

Bug 3

each state is not properly time-travel-ready. Something goes wrong for keyed each blocks in#17033 still. The problem is that fine grained proxy means some array entries may appear undefined for subsequent runs when they shouldn't, but no further insights yet and no idea how to fix yet.

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 withfeat:,fix:,chore:, ordocs:.
  • 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 withinpackages/svelte/src, add a changeset (npx changeset).

Tests and linting

  • Run the tests withpnpm test and lint the project withpnpm lint

dylan1951 and MilosNikolic reacted with heart emoji
@changeset-bot
Copy link

changeset-botbot commentedNov 7, 2025
edited
Loading

⚠️ No Changeset found

Latest commit:34e0ac4

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

@github-actions
Copy link
Contributor

Playground

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

@dummdidumm
Copy link
MemberAuthor

On reflection bug 1 should be solved differently - Rich is gonna take a stab at refactoringeach.js. Only bug 2 should probably land from this PR.

henrykrinkle01 reacted with heart emoji

@Rich-HarrisRich-Harris mentioned this pull requestNov 13, 2025
6 tasks
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Pending async effects breaking synchronous state/effects [bug] Bad rendering using await in component

2 participants

@dummdidumm

[8]ページ先頭

©2009-2025 Movatter.jp