fix: make effects depend on state created inside them #16198
Merged
Conversation
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
|
|
Rich-Harris
approved these changes
Jun 18, 2025
Rich-Harris
pushed a commit
that referenced
this pull request
Jun 18, 2025
* chore: merge main into async branch * adjust test * fix: make effects depend on state created inside them (#16198) * make effects depend on state created inside them * fix, add github action * disable test in async mode
8 tasks
Rich-Harris
added a commit
that referenced
this pull request
Jul 14, 2025
* tidy * tidy * yes it can, apparently * tidy up * unused * complete merge * WIP * simplify * debugging help * WIP * unused * partial merge * WIP * fix * add test * rename * fix * unused * oops * chore: merge main into async branch (#16197) * chore: merge main into async branch * adjust test * fix: make effects depend on state created inside them (#16198) * make effects depend on state created inside them * fix, add github action * disable test in async mode * make batch.#deferred private * fix settled when awaits occur inside pending boundary * tweak * change behaviour of `tick()` to be requestAnimationFrame-based * get rid of a bunch of Promise.resolve chains * more * more * fix test * disallow `flushSync()` inside effects * regenerate * handle errors in block expressions * make validate_each_keys async-aware * for unowned deriveds, throw errors lazily * rename ASYNC_ERROR -> ERROR_VALUE, and avoid conflicts with other flags now that it's used with deriveds as well as sources * invoke boundary directly * local effect pending * update test * fix * fix * fix weird bug in tests * delete old changeset that somehow got left over here * Update .changeset/eleven-weeks-dance.md * update error details * unused * simplify * tweak * tweak * tweak * tweak * tidy up * handle errors in async block expressions * tweak * groundwork for async attribute_effect * dry out * fix async directives * tidy up * initialize option values before initing select values * simplify init_select * simplify * tweak * tidy up * tweak * on second thoughts just simplify it here * tidy * handle awaits in `<slot>` * unused * tidy up * tidy up * dry out * dry out * Revert "dry out" This reverts commit 2585516. * dry out * dry out * use let for block-scoped stuff * dry out * dry out * tidy up * only wrap awaits in `$.save` when necessary * oops * remove TODO comment (just checked) * oops, leftover * simplify * unused * remove logging * tweak * unused * unused * remove logging * partial fix * fix * remove unused EFFECT_HAS_DERIVED * Update packages/svelte/src/reactivity/create-subscriber.js Co-authored-by: Elliott Johnson <[email protected]> * Update packages/svelte/src/index-client.js Co-authored-by: Elliott Johnson <[email protected]> * Update packages/svelte/src/internal/client/runtime.js Co-authored-by: Elliott Johnson <[email protected]> * unused * Update packages/svelte/src/internal/client/reactivity/sources.js Co-authored-by: Elliott Johnson <[email protected]> * Update packages/svelte/src/internal/client/reactivity/deriveds.js Co-authored-by: Elliott Johnson <[email protected]> * Update packages/svelte/src/internal/client/reactivity/deriveds.js Co-authored-by: Elliott Johnson <[email protected]> * prettier * unused * fix flags * tweak * tweak * unused * fix * no idea what a 'boundary micro task' is or why it was deemed necessary but evidently it isn't * remove queue_boundary_micro_task * oops * note * tidy up * remove TODO * make method private * simplify * flesh out await_reactivity_loss warning * tweak * update test * fix * null out from_async_derived in more places * tidy up test * failing test * unused * fix test * fix * simplify. no idea what the async_mode_flag stuff is about, but it appears unnecessary * add async_derived_orphan error * regenerate * flesh out await_outside_boundary message * add some JSDoc * only update `$effect.pending()` if someone is listening, since it causes a double flush and makes debugging harder * tweak logic to make it clearer why and when we commit a batch * add a couple of comments * false -> 0 * add comment * unused * silence warning * add effect_pending_outside_reaction error * Update packages/svelte/src/compiler/types/index.d.ts Co-authored-by: Elliott Johnson <[email protected]> * suspend batch, not boundary * rename from_async_derived -> current_async_derived * tweak * remove TODO - this method is only called when pending snippet exists * use error boundary for test - vitest does some weird error swallowing afaict * flush less often * restore -> activate * remove TODO * move batch-related code into batch.js * make flush_queued_root_effects a method of batch * make process_effects a method of batch * make stuff private * unused * regenerate * update test * more JSDoc * add more JSDoc * branch and block effects do not also need to be render effects * tidy up * simplify * unused * move code where it belongs * remove, for now * fix * only apply error adjustments when error escapes boundaries * remove EFFECT_IS_UPDATING * is_dirty is a better name than check_dirtiness * duplicates are rare and harmless * apparently we no longer need the merging logic? we can simplify and fix stuff by removing it * tidy * don't commit stale batches * add skipped failing test * partial merge * WIP * WIP * WIP * tweak * tidy up * dont update derived status when time-travelling * tidy up * tidy up * tag async deriveds * tweak * bail out of secondary flushes * re-run blocks on subsequent flushes * add test * fix * add tests, one failing * fix * flesh out await_waterfall message * tidy up * dry out * unused * tweak * tidy up * TODO * tweak * tidy up * remove TODO * unused export * add optimisation back * revert unneeded changes * revert * update some tests * more * more * move some code * rename * WIP * unset context synchronously * remove unused argument * Apply suggestions from code review Co-authored-by: Simon H <[email protected]> * add comment * add comment * use queue_micro_task in createSubscriber * Update packages/svelte/src/compiler/phases/3-transform/client/visitors/AwaitExpression.js Co-authored-by: Elliott Johnson <[email protected]> * prettier --------- Co-authored-by: Simon H <[email protected]> Co-authored-by: Simon Holthausen <[email protected]> Co-authored-by: Elliott Johnson <[email protected]>
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.
We want that behavior after all as we have realized - it's vital for thinks like a shared
Resourceclass instance which is scaffolded within the template, shares its instance across call sites, and should still trigger reruns when something changes (which can only happen if the surrounding template will rerun upon changes). Basically a revert of #15553 for effects.Also adds the possibility to run the suite in non-async-mode; we can use that to run a separate github action to make sure we don't accidentally regress.
merges into #16197 which in turn merges into the async branch