Drain queued propchange events on reconnect#120
Merged
DmitrySharabin merged 3 commits intoMay 17, 2026
Conversation
DmitrySharabin
force-pushed
the
fix-propchange-drain-on-reconnect
branch
2 times, most recently
from
5c64952 to
7907d85
Compare
DmitrySharabin
force-pushed
the
fix-propchange-source-first-order
branch
from
70c36ac to
c4e3cf8
Compare
DmitrySharabin
force-pushed
the
fix-propchange-drain-on-reconnect
branch
from
7907d85 to
13367a4
Compare
DmitrySharabin
force-pushed
the
fix-propchange-source-first-order
branch
from
c4e3cf8 to
05ccdb9
Compare
DmitrySharabin
force-pushed
the
fix-propchange-drain-on-reconnect
branch
from
13367a4 to
98d136f
Compare
DmitrySharabin
force-pushed
the
fix-propchange-source-first-order
branch
from
05ccdb9 to
d1d74a5
Compare
DmitrySharabin
force-pushed
the
fix-propchange-drain-on-reconnect
branch
from
98d136f to
e76d161
Compare
LeaVerou
requested changes
May 15, 2026
Contributor
LeaVerou
left a comment
LeaVerou
left a comment
There was a problem hiding this comment.
"drain" reads more like an implementation detail than a user-facing concept. Plus, drain what? It's missing context.
I think it would be useful to make these first-class concepts, with a method to stop dispatching events and another one to resume and drain, and frame around that. I suspect this could also be more efficient, though I haven't seen the code that adds to queue to verify. And I can certainly see that being useful in other situations as well.
DmitrySharabin
force-pushed
the
fix-propchange-drain-on-reconnect
branch
from
b8022d0 to
520b067
Compare
DmitrySharabin
added a commit
that referenced
this pull request
May 15, 2026
Per review feedback on #120: 'drain' reads as an implementation detail and lacks context. The new name says what the method dispatches and that they were queued, matching the existing eventDispatchQueue field.
DmitrySharabin
force-pushed
the
fix-propchange-drain-on-reconnect
branch
from
da9a76c to
25112e1
Compare
DmitrySharabin
force-pushed
the
fix-propchange-source-first-order
branch
from
d1d74a5 to
08e199b
Compare
DmitrySharabin
added a commit
that referenced
this pull request
May 15, 2026
Per review feedback on #120: 'drain' reads as an implementation detail and lacks context. The new name says what the method dispatches and that they were queued, matching the existing eventDispatchQueue field.
DmitrySharabin
force-pushed
the
fix-propchange-source-first-order
branch
from
08e199b to
d1d74a5
Compare
DmitrySharabin
force-pushed
the
fix-propchange-drain-on-reconnect
branch
from
25112e1 to
da9a76c
Compare
DmitrySharabin
added a commit
that referenced
this pull request
May 15, 2026
Per review feedback on #120: 'drain' reads as an implementation detail and lacks context. The new name says what the method dispatches and that they were queued, matching the existing eventDispatchQueue field.
DmitrySharabin
force-pushed
the
fix-propchange-drain-on-reconnect
branch
from
da9a76c to
fd394ef
Compare
DmitrySharabin
force-pushed
the
fix-propchange-source-first-order
branch
from
d1d74a5 to
e163920
Compare
DmitrySharabin
added a commit
that referenced
this pull request
May 15, 2026
Per review feedback on #120: 'drain' reads as an implementation detail and lacks context. The new name says what the method dispatches and that they were queued, matching the existing eventDispatchQueue field.
DmitrySharabin
force-pushed
the
fix-propchange-drain-on-reconnect
branch
from
fd394ef to
f8bedf0
Compare
DmitrySharabin
added a commit
that referenced
this pull request
May 15, 2026
Per review feedback on #120: 'drain' reads as an implementation detail and lacks context. The new name says what the method dispatches and that they were queued, matching the existing eventDispatchQueue field.
DmitrySharabin
force-pushed
the
fix-propchange-source-first-order
branch
from
3625381 to
f7356e7
Compare
DmitrySharabin
force-pushed
the
fix-propchange-drain-on-reconnect
branch
from
f8bedf0 to
afd9438
Compare
DmitrySharabin
added a commit
that referenced
this pull request
May 16, 2026
Per review feedback on #120: 'drain' reads as an implementation detail and lacks context. The new name says what the method dispatches and that they were queued, matching the existing eventDispatchQueue field.
DmitrySharabin
force-pushed
the
fix-propchange-drain-on-reconnect
branch
from
afd9438 to
d2d4b7a
Compare
DmitrySharabin
force-pushed
the
fix-propchange-source-first-order
branch
from
f7356e7 to
5d2dac7
Compare
Base automatically changed from
fix-propchange-source-first-order
to
fix-issue-98
May 16, 2026 12:01
DmitrySharabin
added a commit
that referenced
this pull request
May 16, 2026
Per review feedback on #120: 'drain' reads as an implementation detail and lacks context. The new name says what the method dispatches and that they were queued, matching the existing eventDispatchQueue field.
DmitrySharabin
force-pushed
the
fix-issue-98
branch
from
6b12c36 to
9dd342c
Compare
DmitrySharabin
force-pushed
the
fix-propchange-drain-on-reconnect
branch
from
d2d4b7a to
589ad50
Compare
Member
Author
|
Done — reframed as |
Events fired while the element is disconnected are queued in firePropChangeEvent. Previously, the queue only drained in Props.initializeFor, which runs from the once-per-element `constructed` hook — so events queued during a disconnect→reconnect cycle were lost. Extract the inline drain into Props.drainFor and call it from a new `connected` hook in props/index.js. The hook fires on every connect; on first connect the queue is already empty after initializeFor's drain, so no double-dispatch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Per review feedback on #120: 'drain' reads as an implementation detail and lacks context. The new name says what the method dispatches and that they were queued, matching the existing eventDispatchQueue field.
DmitrySharabin
force-pushed
the
fix-propchange-drain-on-reconnect
branch
from
1b7c407 to
d3109eb
Compare
Replaces the implicit isConnected check with an explicit #paused WeakSet, exposed via pauseEvents/resumeEvents on Props. Lifecycle hooks wire disconnected → pauseEvents, connected → resumeEvents. Foundation for manual batching (#51).
DmitrySharabin
force-pushed
the
fix-propchange-drain-on-reconnect
branch
from
d3109eb to
c1a7bff
Compare
LeaVerou
approved these changes
May 16, 2026
Contributor
LeaVerou
left a comment
LeaVerou
left a comment
There was a problem hiding this comment.
We should coalesce, but this can be done in a separate PR
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.
Summary
Replaces the implicit
isConnectedqueue check with an explicitpauseEvents/resumeEventsprimitive onProps, gated by a#pausedWeakSet. Lifecycle hooks wiredisconnected → pauseEventsandconnected → resumeEvents— same fix, first-class primitive for #51 batching to build on.Addresses Lea's review.
Tests
96/102 PASS locally (4 pre-existing skipped). The original reconnect regression test is unchanged. One new test covers the primitive directly: pause → writes → resume, asserting no dispatch during pause and ordered drain on resume.
The 2 reds (
Post-mount setAttribute…,removeAttribute collapses…) are owned by #117 and will go green once it merges — this PR deliberately doesn't carry that commit.