Skip to content

fix(atoms): clean up MappedSignal#C when no inner signal changes state#345

Merged
bowheart merged 1 commit intomasterfrom
josh/mapped-signal-unchanged-cleanup
Mar 3, 2026
Merged

fix(atoms): clean up MappedSignal#C when no inner signal changes state#345
bowheart merged 1 commit intomasterfrom
josh/mapped-signal-unchanged-cleanup

Conversation

@bowheart
Copy link
Collaborator

Description

I had Claude run through the events system to try to find any bugs. It didn't on the first pass, which is great, but I pressed it and it did find an edge case on the second try. Claude's summary:

Bug Found & Fixed
The bug was in MappedSignal.ts:172 — the this.C (change events) field leaked when .set() was called with a new object reference but all inner signal values were unchanged. Since no inner signals actually changed state, j() was never scheduled and this.C was never cleared. This caused:

  1. Stale events sent on the next update — the leaked this.C would be used instead of the real events from the next state change
  2. Real events suppressed — the !this.C guard in r() prevented forwarding genuine events from inner signals
    The fix — one line added in the finally block of MappedSignal.set():
} finally {
  schedulerPost(this.e)
  this.C = undefined  // ← prevents stale event leaks
}

This is safe because by the time schedulerPost() returns, j() has either already consumed and cleared this.C, or it never ran and the value is stale.

It looks like a real edge case to me and not even that difficult to repro. So I'm PR'ing the fix. Claude added 38 tests before finding this, so I've committed those too.

@bowheart bowheart merged commit 5363bfc into master Mar 3, 2026
2 checks passed
@bowheart bowheart deleted the josh/mapped-signal-unchanged-cleanup branch March 3, 2026 23:08
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