Master signals fix ged#1884
Merged
Merged
Conversation
…date Previously, when a signal was written, updateComputation(effect) walked effect.sources eagerly in its PENDING branch and recomputed every upstream computed before the effect's body ran. This was asymmetric with the mount path (which is lazy) and surprising when debugging via computed-side logs. markDownstream now marks downstream effects as STALE (not PENDING) so they skip the eager-source loop and re-execute directly, pulling computeds as they're read. Computed-to-computed PENDING propagation is preserved, so chained computeds still skip recomputation when their input values are unchanged. Trade-off: effects now re-run when any transitive signal source is written, even if intermediate computeds collapse to the same value. The previous effect-skip optimization (asserted by tests/computed.test.ts:194 and :337) is gone; the tests now document the new contract. Doc reactivity.md gains an "Evaluation order" subsection spelling this out. A try/finally around currentComputation in updateComputation ensures the tracking pointer is restored if compute() throws.
…fect is not re-run
When `updateComputation(effect)` inside processEffects threw, the for-loop
exited before the trailing `observers.length = 0` could run, leaving the
queue populated. Any re-entrant `batchProcessEffects()` from the same
flush (e.g. via `onWriteAtom` inside a computed wrapper whose value
actually changed) then scheduled a second microtask that re-processed the
same effect — observable in the playground reproducer
const list = signal(["a"]);
const lastValue = computed(() => list().at(-1));
const uppercase = computed(() => lastValue().toUpperCase());
effect(() => { console.log("trigger effect"); uppercase(); });
list.set([]); // makes uppercase throw
as "trigger effect" being logged twice (and uppercase throwing twice)
after a single signal change.
processEffects now swaps the queue with a fresh array before iteration, so
the throw can't leak stale entries. Effects queued during the current
flush still land in the (now-fresh) shared `observers` and are processed
by the next microtask, which preserves the batching invariant.
Module-level mutable state made this leak across tests: any test that
left a non-empty `observers` (because an earlier test triggered the throw
path) would pollute subsequent tests in the same file. Adding the new
regression test surfaced this — without the swap, 7 unrelated effect
tests fail.
The regression test deliberately lets the throw propagate through batched()'s
.then() chain. It uses a named IntentionalTestError class so vitest.config.ts's
`onUnhandledError` callback can filter just that one rejection instead of
blanket-silencing every unhandled error in the suite.
ged-odoo
force-pushed
the
master-signals-fix-ged
branch
from
2ab105d to
9820f7e
Compare
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
2 participants
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.
No description provided.