Skip to content

Failing test for #19344 - #21530

Open
johanrd wants to merge 3 commits into
emberjs:mainfrom
johanrd:todo-test-gh19344
Open

Failing test for #19344#21530
johanrd wants to merge 3 commits into
emberjs:mainfrom
johanrd:todo-test-gh19344

Conversation

@johanrd

@johanrd johanrd commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Adds a failing test for #19344: with autorun enabled (as in apps), a state change in the first {{on}} listener schedules the render flush as a microtask, and browser-dispatched events run a microtask checkpoint between listener callbacks — the flush tears down the element's modifiers mid-dispatch and the removed second listener is never invoked. Test-mode clicks mask this (runloop-wrapped, autorun disabled), which is why it went uncaught; the test uses setTesting(false) and a browser-task scroll dispatch to reproduce the real path.

The test is a QUnit todo: CI stays green while the bug exists, and fails once it is fixed (then flip @todo to @test).

Mechanism (verified on main): each {{on}} registers its own native listener (on.ts#L220). A state change in the first listener schedules the render flush as a microtask (backburner autorun). For browser-dispatched events the JS stack empties between listener callbacks, so a microtask checkpoint runs between the two listeners — the flush tears down the element's modifiers, teardown calls removeEventListener (on.ts#L47-L53) on the not-yet-invoked second listener, and a listener removed mid-dispatch is never invoked.

Why tests never caught it (@marcin-wicha's false positive): test mode masks it twice — click() is runloop-wrapped and autorun is disabled — and even with autorun enabled, synthetic el.click() dispatches inside the calling JS stack, so no checkpoint runs between listeners. Only browser-task dispatch reproduces it. #21530 adds a failing (todo) test using setTesting(false) plus a browser-dispatched scroll.

Fix prototype: johanrd#38 — all {{on}} handlers for the same (element, event, capture/passive bucket) share one native listener that dispatches to a snapshot of the handler list, so sibling handlers run in a single callback frame and no checkpoint can fall between them. Full suite green (9437 tests, 0 failures); the failing test flips to passing. Scope note: this covers the reported same-element case. A cross-element variant exists (a handler's flush destroying an ancestor's {{on}} before the event bubbles up to it) — fixing that requires deferring teardown past the dispatch, i.e. deciding whether handlers may fire after their component is destroyed, which needs maintainer input.

Precedent — every mainstream framework funnels its handlers through a single native callback per dispatch; the surfaces that don't are exactly where this bug class lives:

architecture source
React one native listener per event at the root, synthesized propagation, post-dispatch flush DOMPluginEventSystem.js#L432-L435
Vue 3 per-element invoker (el[veiKey]), handler swaps mutate invoker.value, stale-event guard via e._vts <= invoker.attached events.ts#L43-L61, #L112-L116
Solid document delegation, handlers as node[$$event] props client.js#L90-L96
Svelte 5 root delegation — and the same bug class, open, at the native/delegated boundary: sveltejs/svelte#18070 events.js#L173-L177
Angular per-listener change detection by default; opt-in rAF event coalescing exists precisely for multi-handler events ng_zone.ts#L310-L328

Cowritten by Claude

johanrd added 2 commits July 28, 2026 08:44
Disabling a focused element fires blur synchronously during render, so a
{{on "blur"}} listener that updates rendered state trips the backtracking
assertion. Recorded as a QUnit todo test (via new @todo support in
moduleFor) so it passes CI while the bug exists and flags when fixed.
With autorun enabled (as in apps), a state change in the first {{on}}
listener schedules the render flush as a microtask, and browser-dispatched
events run a microtask checkpoint between listener callbacks. The flush
tears down the element's modifiers mid-dispatch, and the removed second
listener is never invoked. Uses setTesting(false) plus a browser-task
scroll dispatch to reproduce the real path; synthetic clicks in test mode
mask the bug (both listeners fire), which is why this went uncaught.
@johanrd
johanrd force-pushed the todo-test-gh19344 branch from 52caf45 to 8423c82 Compare July 28, 2026 06:53
The bug it documents is the backtracking assertion, which is stripped in
production. There the blur listener succeeds, the todo passes in full, and
QUnit reports "expected todo to not pass" — failing the Production build
jobs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants