ReviewOS
@if (!issue)

No such issue

stacks/stx has no issue numbered 2016.

@else

stacks/stx

@error and @load are not wired, so Avatar's image fallback never fires

#2016
Closed glennmichael123 opened this 9 days ago ·
9 days ago

@error and @load are not wired as DOM listeners, so Avatar's own image fallback never fires. The attribute renders into the HTML, which makes it look wired.

The whitelist

dist/runtime-globals.js holds 25 runtime event names:

click dblclick mousedown mouseup mousemove mouseenter mouseleave
keydown keyup keypress input change submit focus blur scroll resize
touchstart touchend touchmove contextmenu wheel pointerdown pointerup pointermove

error and load are both absent. In binding-manifest.js, an @name whose bare name is not a directive and not in that set falls through to return "attr", so it is treated as a plain attribute rather than an event binding.

What that breaks

src/ui/avatar/Avatar.stx:

const showImage = derived(() => hasSrc && !imageError())
...
<img :show="showImage()" @error="onImageError($event)">
<span :show="!showImage()">{{ displayInitials }}</span>

imageError() can never become true, so showImage() is hasSrc forever. An avatar whose URL 404s or 403s renders an empty circle and the initials never appear — exactly the case the fallback exists to handle.

This is the component's documented selling point over a hand-rolled , so a failed load uncovers what is already there.

9 days ago

Fixed in 071e3a0b12.

Your closing paragraph is the fix rather than the two names: @ means "event listener" in the prefix convention, so a whitelist was the wrong shape for it. An @name that is not a directive is now an event whatever the name — which covers error and load and everything else you listed in the same position: animationend, transitionend, drop, dragover, paste, copy, the <dialog>/<details> events, the media events.

A typo now binds a listener for an event that never fires — visible in devtools, costing nothing — rather than being silent dead markup. That is the failure you asked to stop being invisible, and it seemed better to remove the category than to make it warn.

Why I did not add them to the whitelist

That was the suggested fix and it would have broken two components. RUNTIME_EVENT_NAMES is shared between @name and :name, and : is the structural and prop prefix — <EmailInput :error="error"> and <SearchInput :error="error"> pass error as a prop. Adding error to the set makes :error an event binding and both inputs stop receiving their error state: a broken avatar traded for a broken form.

So :name keeps the whitelist and only @name changed. There is a test asserting error and load are still absent from RUNTIME_EVENT_NAMES, with that reasoning attached, so the obvious fix is not re-applied later.

One correction to the report

In binding-manifest.js, an @name whose bare name is not a directive and not in that set falls through to return "attr"

Right, and that was the whole bug — but the runtime's own attribute walk has always bound any @name as a listener, without consulting the event set. So only the manifest path disagreed, which is why the attribute looked wired and sometimes was, depending on which path bound the element. The two agree now and nothing in the runtime changed.

Your workaround is still worth keeping for anyone on an older version, and the shape you chose — fallback content underneath rather than in an