aurelia / binding

A modern databinding library for JavaScript and HTML.
MIT License
111 stars 96 forks source link

click.delegate bindings intermittently execute more than once in an Aurelia application #793

Open massimocode opened 2 months ago

massimocode commented 2 months ago

I'm submitting a bug report

Please tell us about your environment:

Current behavior: We have a front-end application built using Aurelia. We have some automated UI tests for it. Sometimes our UI tests will fail. We added much more in-depth logging and found that it was because buttons that had the click.delegate binding on it were executing the handler more than once when they were clicked. So our test was clicking the element once, but the handler was running more than once.

When we changed those handlers to use click.trigger, we could no longer reproduce it.

We have also seen this behaviour in our production application, where users who we believe to be normal users (as we looked at their analytics sessions and didn't see any bot like behaviour) were submitting forms multiple times in extremely quick succession, impossibly quick for a user.

Expected/desired behavior: When click.delegate is used, the handler should be called only once. (Most of the time this is the case, but as I mentioned there see to be cases where it is not).

I could not replicate this using a simple gist (1 App viewmodel, no routing) but I have a repository with tests that fail quite reliably when run multiple times.

I'm looking forward to hearing your thoughts and trying anything that you'd like me to try.

The application is 4 years old and this issue has existed for pretty much as far back as I can remember. I also remember seeing this issue whilst working on another Aurelia application as far back as 2016.

bigopon commented 2 months ago

Thanks @massimocode , ill see if I can guess-locate the code without a repro.

bigopon commented 2 months ago

Those I should also say that click.delegate is no longer recomended, as the pros are longer worth the cons.

massimocode commented 2 months ago

Thanks for confirming @bigopon. In that case we will switch over to click.trigger instead. We don't use it heavily with hundreds/thousands of elements on a page anyway and I think that was the main driving factor for click.delegate.

It might be worth a quick update to the docs that either delegate shouldn't be used anymore or perhaps should only be used with many repeated elements or something.

I'm happy to try anything you want (i.e. add logging statements or amendments) to help look into this issue if you wish to pursue it.

bigopon commented 2 months ago

Yes, we should do that. Ccing you @Vheissu for the note in our new v1 doc.

bigopon commented 2 months ago

I'm happy to try anything you want (i.e. add logging statements or amendments) to help look into this issue if you wish to pursue it.

Thanks, if I do I will try to ask for help when needed 🙏 .