Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inconsistency of solid/reactivity rule when dealing with event handlers? #168

Open
pedro00dk opened this issue Nov 13, 2024 · 1 comment
Assignees
Labels
question Further information is requested

Comments

@pedro00dk
Copy link
Contributor

pedro00dk commented Nov 13, 2024

Hello, I have been using solid-js for more than two years and recently decided to use this tool.
I event recently made a small patch regarding one of the issues: #164

However, in my process of fixing linting issues I kept finding some cases that are inconsistent and IMHO could be handled differently.
I made a list in the playground to identify all cases: https://playground.solidjs.com/anonymous/29d817a1-ae6e-419e-af52-01ccc5287064

image

Besides the issues from my patch, I found:

image

  • These two should definitely trigger the solid/reactivity rule


image
These are the tricky ones. I understand function attributes of Components are expected to be synchronous, as they might access signals/stores, and be called arbitrarily inside the component.
However, at the same time, I believe it is not a stretch to expect attributes named like on[A-Z][a-z_$]* to also be used as event listeners, even if passed to Components. In most cases, these would actually called as subroutines of event listeners of native elements....
Right now, there are already special rules such as static attribute prefixes. Wouldn't be valid to also consider Components' function attributes which name looks like on[A-Z][a-z_$]* to also be especial cases?

@pedro00dk pedro00dk added the question Further information is requested label Nov 13, 2024
@joshwilsonvu
Copy link
Collaborator

@pedro00dk Thanks again for your last patch and for doing the investigation here! These are some good points—I do think some regexes need to be adjusted to be as permissive as possible.

  • I'm with you that the async functions passed as props to <Button> should not warn.
  • For
    <button onclick={props.onClick} />, // WRONG - complain only about attribute casing
    I think one warning is enough. Though onclick is certainly intended to be an event handler, it could just as easily be something like only or once, hence the existing rule guiding the user to make their intention explicit. See only detected as event handler #23
  • For
    <button on-click={props.onClick} />, // WRONG - handler is only attached once (even if custom event)
    I didn't even know the dash (-) syntax was legal! I'm happy for this to be treated like other event handlers.

I'd welcome a PR for these, but no obligation. I can take it as time allows if you don't get to it first. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants