Skip to content

fix(nanoviews): fire every on* prop, not just the bubbling ones - #204

Merged
dangreen merged 1 commit into
mainfrom
fix/nanoviews-native-event-listeners
Aug 20, 2026
Merged

fix(nanoviews): fire every on* prop, not just the bubbling ones#204
dangreen merged 1 commit into
mainfrom
fix/nanoviews-native-event-listeners

Conversation

@dangreen

Copy link
Copy Markdown
Member

onFocus did not fire. Neither did onBlur, onMouseEnter, onScroll, onPlay, any media event, load, error, any *Capture prop, or onDoubleClick. All of them are in the type surface, all of them were accepted without complaint, and none of them ever called anything. Verified in Chrome 151 — of seven handlers put on one <input>, exactly one fired:

{"delegated": {"click": 1}, "direct": {"focus": 1, "blur": 1, "mouseenter": 1, "scroll": 1, "play": 1}}

The second row is plain addEventListener on the same element in the same test: the events happen, delegation just never saw them.

Why

Handlers were delegated — one document.addEventListener(type, dispatcher) per event type, the handler stashed on the element as element.__click, and dispatch walking composedPath() from the target up. A listener on document in the bubble phase cannot see an event that does not bubble, and focus, blur, mouseenter, mouseleave, scroll, load, error and the media events do not. *Capture failed differently: name.slice(2).toLowerCase() turns onClickCapture into the event "clickcapture", which does not exist. onDoubleClick became "doubleclick"; the DOM spells it dblclick.

What replaces it

The listener goes on its own element:

function setEventListener(element: Element, name: string, value: TargetEventHandler) {
  // `onGotPointerCapture` and `onLostPointerCapture` end with `Capture`
  // themselves, and are ordinary bubbling events
  const capture = name.endsWith('Capture') && !name.endsWith('PointerCapture')

  element.addEventListener(
    eventNames[name] ??= name.slice(2, capture ? -7 : undefined).toLowerCase(),
    event => untracked(() => (value as EventListener).call(element, event)),
    capture
  )
}

That is the whole event system. internals/elements/events.ts is deleted, along with defineProtoProp and the __mp mount marker whose only job was stopping the dispatcher's walk. The untracked wrapper stays and is load-bearing: autoFocus$ calls focus() from inside an effect, and without it a handler reading a signal would subscribe that effect.

controls.ts binds through the same call. A registration dies with its element, so a control binding needs no teardown — and therefore no effect node to carry one, which is what the old code used a dependency-less effect for. The value$-versus-onInput collision disappears with the single slot they used to share: the browser holds any number of listeners per element and event, and runs them in registration order.

Two more fixes fall out of being on the element instead of on document:

  • preventDefault() inside onWheel, onTouchStart and onTouchMove now works. Document-level listeners for those types are passive by default, so Chrome was discarding it — measured defaultPrevented: false before, true after.
  • A third-party stopPropagation() below the document no longer silences an element's own handler. Probed in Chrome: the button's own onClick runs, the ancestor's does not, which is what the DOM promises.

onDoubleClickonDblClick

The prop has never fired, so nothing can be depending on it, and the name it should have had is the one the event actually has. Solid spells it the same way. Keeping the old spelling would mean carrying a permanent special case in the hot path for a prop that was born broken.

Cost, measured

Attaching is not free the way writing a slot was. In Chrome, ns per handler, arms interleaved round-robin, 31 reps × 20 000 elements, order rotated per rep:

ns
the old slot write 20
naive addEventListener with a freshly built name 250–255
as written here 195–205

The gap between the last two is not the parsing — that is 20 ns. It is the browser atomising a string it has not seen as that object before; memoising the event name per prop name hands back the same object and returns about a fifth of the path. Map instead of an object, caching capture alongside the name, splitting into two objects, deriving capture from the name length: all measured, all within noise of each other on speed, all 6–17 B larger.

js-framework-benchmark, 15 iterations per arm, back to back on an idle machine:

before after
create 1k 101.1 101.3 +0.2%, P = 0.58 — the stand's own noise floor is P ≈ 0.57
create 10k 876.7 894.7 +2.1%, P = 0.82

The 10k case is a genuine cost and it is not the attach path — the arithmetic there accounts for about 4 ms of the 18. The rest is the browser's own bookkeeping for twenty thousand live listeners. Set against it: dispatch is roughly four times cheaper than the deleted dispatcher (423–488 ns of handler-attributable work against 1808–1905), so one dispatched event pays back seven attaches. Ten thousand rows each carrying a handler is a benchmark shape; events happening is an application shape.

Size

createElementPropertySetter also gains /* @__NO_SIDE_EFFECTS__ */. It was called at module root without it, so the bundler had to keep the call and everything it reached — importing value$ alone was shipping the checked$ and selected$ implementations. That one line is worth 285 B on its own, and it was true before this change too.

gzip before after
all publics 7594 7401
average usage 4249 3790

All four pins come down.

Tests

Six added: a non-bubbling event fires, capture ordering runs outer-capture → target-capture → target-bubble, onDblClick reaches dblclick, onGotPointerCapture is not mistaken for a capture handler, a handler dispatched from inside an effect does not subscribe that effect, and a control binding coexists with a handler for the same event in either key order. 109 pass; the five fixes are also verified against the built dist in Chrome 151.

Handlers were delegated: one listener per event type on `document`, and the handler stashed on the element in a `__type` slot. Events that do not bubble never reach a document listener in the bubble phase, so `onFocus`, `onBlur`, `onMouseEnter`, `onScroll`, `onPlay`, the media events and `load`/`error` were typed, accepted and silently dead. So was every `*Capture` prop - the prop name was lowercased whole, and `"clickcapture"` is not an event. So was `onDoubleClick`, for the same reason: the DOM spells it `dblclick`.

Handlers now go on their own elements. The dispatcher, the prototype slots and the mount marker that stopped its walk are gone, and `controls.ts` binds through the same call - a registration dies with its element, so a control binding needs no teardown and no effect node to carry one. `value$` and an `onInput` handler no longer fight over one slot: the browser holds any number of listeners per element and event.

Two more things follow from being on the element rather than on `document`. `preventDefault()` in `onWheel`, `onTouchStart` and `onTouchMove` works - document listeners for those types are passive by default and the browser was ignoring it. And a third-party `stopPropagation()` below the document no longer silences an element's own handler.

`onDoubleClick` becomes `onDblClick`, which is what the event is called; the prop has never fired, so nothing can depend on it. Event names are memoised per prop name - the browser atomises a freshly built string on every registration, and handing back the same string object is worth about a fifth of the attach path.

`createElementPropertySetter` gains `@__NO_SIDE_EFFECTS__`. It was called at module root without the annotation, so a bundler had to keep it and everything it referenced: importing `value$` alone shipped the `checked$` and `selected$` implementations too.
@codecov

codecov Bot commented Aug 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.32%. Comparing base (9fc639f) to head (b1d6290).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #204      +/-   ##
==========================================
+ Coverage   85.29%   85.32%   +0.03%     
==========================================
  Files         141      139       -2     
  Lines        3168     3142      -26     
  Branches      593      591       -2     
==========================================
- Hits         2702     2681      -21     
+ Misses        335      332       -3     
+ Partials      131      129       -2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@dangreen
dangreen merged commit 42e613b into main Aug 20, 2026
10 checks passed
@dangreen
dangreen deleted the fix/nanoviews-native-event-listeners branch August 20, 2026 13:20
@github-actions github-actions Bot mentioned this pull request Aug 19, 2026
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