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

Any element attributes that are functions are added as event handlers #2592

Closed
speckins opened this Issue May 16, 2018 · 3 comments

Comments

Projects
None yet
4 participants
@speckins

speckins commented May 16, 2018

  1. Describe your issue:

I ran into this issue while developing a tag where I wanted to pass in a function in its opts.

<parent-tag>
  <search-tag search={ my_search_fn }></search-tag>

  this.my_search_fn = function(query) { return $.get(/*...*/, {q: query}) }
</parent-tag>

The function itself is not an event handler -- it accepts a string -- but is intended to be included in one within the tag.

The issue is that the function seems to be added to the <search-tag> as an event handler for the "search" event. I would not have expected this unless I used the "onsearch" attribute. I suspect that Riot adds any attribute with a function value as an event handler regardless of the attribute name.

I'm not sure that this is a bug but should Riot only attach event handlers for valid event attributes, i.e., only for attribute names matching "on*"? If I did this outside of Riot, <div click="alert('Hello')">, the browser would not fire the alert because the correct attribute is onclick, not click.

  1. Can you reproduce the issue?

https://plnkr.co/edit/LmeTBmm4m6EAHkmiRZao

I used the "click" event instead of the "search" event in Plunker. You'll need to open the console, and you'll see both the string and the event logged each time the button is clicked.

  1. On which browser/OS does the issue appear?

I observed this in Chromium and Opera on macOS.

  1. Which version of Riot does it affect?

I'm using Riot 3.6.3, but the Plunker above is the latest (3.10.0).

  1. How would you tag this issue?
  • Question
  • Bug
  • Discussion
  • Feature request
  • Tip
  • Enhancement
  • Performance
@sourcegr

This comment has been minimized.

Contributor

sourcegr commented May 16, 2018

Yea, I have seen this bug before.
It was not there on the riot@2

This bug was introduced with riot@3

@GianlucaGuarini GianlucaGuarini added bug and removed to verify labels May 16, 2018

@GianlucaGuarini

This comment has been minimized.

Member

GianlucaGuarini commented May 16, 2018

that's a bug that I will fix ASAP. A fix should be really simple thanks for submitting this issue

@ericmorand

This comment has been minimized.

ericmorand commented Jul 25, 2018

@GianlucaGuarini you created a breaking change for all the people that were using that feature. If attributes handled by a function were considered as event handler by your test suite, you are not allowed to change this in a patch or even a minor version.

How can we rely on your library if you create breaking changed outside of major versions? A lot of our projects were based on that feature and we now have to rewrite all of them because we were handling the click event using the click attribute. Which was working with riot@3.10.0 and thus was validated by your test suite and thus was set in stone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment