Merged
Conversation
🦋 Changeset detectedLatest commit: a50d0ec The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
Contributor
|
Member
Author
|
moving back to draft, some more improvements incoming |
Member
Author
Contributor
elliott-with-the-longest-name-on-github
left a comment
There was a problem hiding this comment.
This looks awesome. I'll wait for Simon to review as well before approving; I think everything is good but given he's more familiar with the clientside stuff he should definitely take a look
Comment on lines
-18
to
-27
| // TODO why is this necessary? why isn't `await tick()` enough? | ||
| await Promise.resolve(); | ||
| await Promise.resolve(); | ||
| await Promise.resolve(); | ||
| await Promise.resolve(); | ||
| await Promise.resolve(); | ||
| await Promise.resolve(); | ||
| await Promise.resolve(); | ||
| await Promise.resolve(); | ||
| flushSync(); |
Contributor
There was a problem hiding this comment.
😆 I saw this while I was working on async SSR and I was really hopeful we could murder it at some point
dummdidumm
reviewed
Sep 30, 2025
…to merge-batches
Merged
dummdidumm
added a commit
that referenced
this pull request
Oct 14, 2025
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
6 tasks
dummdidumm
added a commit
that referenced
this pull request
Oct 14, 2025
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
Rich-Harris
added a commit
that referenced
this pull request
Oct 14, 2025
* 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>
Rich-Harris
added a commit
that referenced
this pull request
Oct 18, 2025
* 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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is part of a project to update some of the async batching logic to handle a few edge cases that have cropped up.
The observable change here is that if you have an
awaitexpression that runs twice, and the second one somehow resolves before the first, they are no longer forced to resolve in linear order. The easiest way to see it is in this example — clicka++thenb++in quick succession:main— the fastb++update is blocked on the slowa++updateb++update occurs, followed by thea++updateTo make this work, whenever a batch is committed, we essentially 'rebase' other pending batches on top of the newly applied state. In the example above, when we commit the fast
b++update, we re-run any async effects that depend onbin the context of the pendinga++batch.Before submitting the PR, please make sure you do the following
feat:,fix:,chore:, ordocs:.packages/svelte/src, add a changeset (npx changeset).Tests and linting
pnpm testand lint the project withpnpm lint