gmac / backbone.epoxy

Declarative data binding and computed models for Backbone
http://epoxyjs.org
MIT License
615 stars 89 forks source link

Correct value compare on set #90

Closed STAH closed 10 years ago

maxpoulin64 commented 10 years ago

I don't think strict comparison makes any sense here. $element.val() always return a string even for <input type="number" />, so if your model stores integers this will always evaluate to false and will needlessly update the element all the time.

Is there a particular use case where you would really need a strict comparison over a form element's value?

STAH commented 10 years ago

Yes, but not if value=0 and $element.val() is empty.

Try in Chrome console:

""===0 --> false ""==0 --> true

maxpoulin64 commented 10 years ago

Indeed, you are right. Maybe you can get rid of the entire condition as well: I just checked the "text" handler and it just replaces it straight away. I'd expect the text handler to be heavier and more used than the value one so it's probably not even worth bothering to check before updating as form values are usually pretty small and can't even cause reflows or anything.

gmac commented 10 years ago

Good catch, I see what you're saying. We do want to avoid setting values when not actually necessary to avoid feedback loops, so maybe adjusting the check to cover that falsey value test would do the trick. How about casting them both as strings and testing strict equality, such as:

if (String($element.val()) != String(value)) $element.val(value);

Please see if that (or something similar) does the trick, then update your pull request and I'll happily merge it.

STAH commented 10 years ago

Opened another PR #91 with correct cast (optimized)