only detected as event handler

[edemaine]

Describe the need The Solid compiler can have some subtle weird behavior around attributes starting with on. It uses heuristics to detect whether the attribute is an event handler (like onclick) or a regular attribute (like only). Examples:

• <button only={count()}> is detected as an event handler. Playground
• const string = 'foo' + count(); <button only={string}> is detected as an event handler. Playground
• const string = 'not event'; <button only={string}> is detected as an attribute. Playground

Suggested Solution I think the simplest way to avoid confusion here is to use camelCase attributes for event handlers, and maybe attr:only to force regular attributes. It's much harder to read onLy as something other than an event handler, so it has explicit intent. Some possible rules:

• If Solid would detect an attribute as an event handler, but it doesn't have a capital letter after on, then suggest either making that letter capital or using attr: if it's actually supposed to be an attribute.
• If Solid would detect an attribute as not an event handler, and it's a native element (single lower-case word like <div>), then suggest using attr: prefix to avoid ambiguity. (Maybe less important.)
• Suggest replacing onclick etc. with onClick, for an explicit whitelist of events. (Not sure this is super important though. It's similar to class vs. className, though a difference is that className is soon to be deprecated.)

Additional context Solid's event handler detection code. This gets applied whenever the attribute starts with on.

Based on recent discussion on Discord.

[joshwilsonvu]

Thank you for the issue and suggestions!

Let me confirm I understand everything: for native elements (single word, all lowercase) only, Solid runs a confusing heuristic based on the statically-determined type of value, and considers some values event handlers. Is this only when the third letter is lowercase, or for any case starting with on?

I'd like to fix up code such that there's never doubt whether a prop is an event listener or an attribute, even if you've never heard of this heuristic, so I'm not yet sold on varying the rule's behavior on "if Solid would detect an attribute as an event handler". I can check, but I strongly doubt that there are any native element attributes that begin with on that aren't event handlers. That makes me think that any native element prop starting with on should either have an uppercase third letter to force it to be treated as an event handler (with an autofix provided for a list of known events), or be explicitly marked with attr:. And nothing would need to be done for Solid or web components if I understand correctly. Do you see any problems with that?

[edemaine]

Is this only when the third letter is lowercase, or for any case starting with on?

Unfortunately, it's for all attributes starting with on

onfoo and onFoo are treated exactly the same, thanks to this helper.

I can check, but I strongly doubt that there are any native element attributes that begin with on that aren't event handlers.

I think this is correct. Or more precisely, there are no HTML and no SVG attributes at all starting with on. (onClick isn't an attribute in the SGML sense; it's a DOM property.)

Unfortunately, web components are a problem. Solid treats <my-component only={count()}> as an event handler (Playground). This is maybe a questionable feature in Solid. But it does look like web components support event handlers, as they subclass HTMLElement, so it does make sense. (When I said "single word", I meant not things like foo.bar which are treated as Solid components (Playground).)

But I think it would be reasonable to forbid onfoo and require either onFoo or attr:onfoo. The former will still break if you intended it to be an attribute, but I think it's clear from the code writing that it's intended to be an event handler.

[joshwilsonvu]

Thanks for the detailed explanation and the links to source code, that's very helpful. Here's where I think I'm landing.

For native HTML and SVG elements and web components:

1. Forbid onfoo and suggest either onFoo or attr:onfoo, also allowing on:foo.
2. For on* attributes without attr:, replicate Solid's detection heuristic. If Solid doesn't think it's an event handler, warn that the user's expectations are broken and suggest a fix such as wrapping with a function to satisfy the heuristic.

With this in effect, any prop that starts with on will be an event handler, and anything else will be a normal attribute.

For Solid components, I'll warn against using attr: or other prefixes. They don't do anything but change the prop name (playground), and could cause confusion. Not critical, just a little bonus.

[edemaine]

Sounds great to me!

You mentioned on:foo. Just an FYI that these are always treated as event handlers, and that they behave differently tham onfoo/onFoo for a special set of delegated events. (You probably know this.) Anyway, yeah, no need to warn in this case. (Just don't want to suggest to users to migrate to this.)

[joshwilsonvu]

Right, yeah, those will use HTMLElement.prototype.addEventListener directly instead of Solid's delegation helper, and I'll leave it alone for power users but won't mention it in warnings. No worries!

[joshwilsonvu]

I took a closer look at the code and learned a few interesting things.

1. Solid's array syntax for event listeners (onClick={[onClick, expr]}) only has an optimization for delegated events, and for other events it's compiled to onFoo={e => onFoo(e, expr)}, creating an additional closure.
2. The detectResolvableEventHandler function you mentioned seems to be only used for choosing between _el$1.addEventListener("ly", () => props.only) and addEventListener(_el$1, "ly", props.only), i.e. subtle differences in regards to event delegation, but still an event handler either way.
3. I think the explanation for the behavior you've seen is actually here: the Babel plugin tries to statically evaluate the expression, and if it can tell that it's a string or number, it puts it in the template as an attribute instead of attaching it as an event handler. That's why it's smart enough to detect const string = 'not ' + 'event' but as soon as there's a function call in the expression it no longer treats it as a constant string/number, or therefore an attribute.

Points 1 and 2 are just technical details and don't need to be user facing, but point 3 is crucial for a lint rule. Fortunately, it's simple to emulate with ESLint, I can statically evaluate expressions the same way.

So—good news! I've published v0.5.0 of the plugin that replicates Solid's logic and tries to make this whole situation a little bit more sane. Any feedback is welcome!

[edemaine]

Awesome, thanks for looking into this further and implementing! I'm always learning more about how Solid works...