fix(react-call): end-all no longer drops same-tick calls#96
Merged
Conversation
An end-all `end()` (no target promise) scheduled a deferred stack-clear that re-used `null` as a "match everything" sentinel: `store.remove(null)` filtered with `promise && c.promise !== promise`, which is falsy for every item, so it wiped the whole stack on the next macrotask. Any `call()` or `upsert()` issued after the `end()` in the same synchronous tick was appended and then clobbered — `X.end(); X.call(...)` rendered nothing. createEnd now captures the exact set of promises it resolves during its synchronous `set()` pass and defers removal of only those, so calls added later survive. The `null` "clear all" branch is gone from the store: `remove` takes a `Set<Promise>` and filters with `!promises.has(...)`. Targeted `end(promise, value)` (a one-element set) and `update()` (which never scheduled a removal) are unchanged. - index.tsx: capture resolved promises in createEnd; pass the set to remove. - store.ts: remove(promises: Set<Promise>) instead of the null sentinel. - end.test.tsx: regression block `same tick as later call()` covering end-all, end-all with pre-existing calls, targeted end, and update — flushing the unmountingDelay macrotask before asserting. - ADR-0020 records the scoped-removal decision; changeset filed as patch.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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.
What
Fixes a real library bug: calling the end-all form
X.end()(no target promise) and thenX.call({...})one or more times in the same synchronous tick resulted in the new calls never rendering — the stack ended up empty even thoughcall()ran afterend(). No error was thrown.Reproduced in the web gallery (React 19) while building examples; the
broadcast-updateexample had been rewritten to avoid the pattern as a workaround.Root cause
The end-all path used
nullas a "match everything" sentinel for its deferred removal:So
end()scheduled a blanket "clear the entire stack" for the next macrotask. Calls added afterend()in the same tick were appended first, then wiped when the deferredremove(null)fired. Removing the leadingend()removed the scheduled wipe — hence the calls rendered.The targeted path (
end(promise, value)) was never affected: it only filters out the one matching promise.update()(update-all) was never affected either: it mutates in place and schedules no removal.The fix
createEndnow captures the exact set of promises it resolves during its synchronousset()pass, and defers removal of only those. Thenullsentinel is gone from the store —removetakes aSet<Promise>and filters with!promises.has(c.promise). Calls added later in the same tick are never in the captured set, so they survive. Targetedend()passes a one-element set and behaves exactly as before.See ADR-0020 for the decision and the rejected alternatives.
Tests
New regression block
same tick as later call()inend.test.tsx, covering all three interactions called out during triage — each flushes theunmountingDelaymacrotask before asserting (the bug only manifested after the deferred removal ran):call()×3 → 3 render (the exact repro; was 0 before the fix)end(promise)thencall()→ the targeted call is removed, later ones surviveupdate()thencall()→ all surviveFull suite green: 132 unit + 5 dist tests,
tsc -bclean, biome clean,size-limitholds (main.js810 B /main.cjs895 B, both under the 1 KB budget).Release
Patch changeset for
react-callincluded, so merging tomainopens/updates the "Version Packages" PR via the release workflow.