A more modest valHooks proposal #295

Merged
merged 3 commits into from Apr 10, 2011

Projects

None yet

6 participants

@timmywil
Member
timmywil commented Apr 2, 2011
  • The main difference is that this does not allow arbitrarily adding hooks to any collection of elements.
  • Modularizes val into a set of easily maintainable and conditional hooks.
  • valHooks is placed at jQuery.valHooks
    • This could technically be extended, but I do not see it being used except in very rare cases since you can only apply valHooks to nodeNames and input types, and not a collection of elements as before. We could theoretically privatize valHooks taking it off of jQuery and only use it internally for our own convenience, but again, I do not believe this patch carries with it the dangers of the first proposal.
  • Slightly improved performance of val on radios and checkboxes for browsers that support checkOn, given the conditional attachment of its hook.

Performance

  • http://jsperf.com/valhooks-vs-val
    • There is a noticeable improvement in getting the value
    • Setting looks about the same, maybe a slight decrease, probably due to accessing nodeName and type on the elem.
timmywil A more modest valHooks proposal
- The main difference is that this does not allow arbitrarily adding hooks to any collection of elements.

- Modularizes val into a set of easily maintainable and conditional hooks.

- valHooks is placed at jQuery.valHooks

  + This could technically be extended, but I do not see it being used except in very rare cases since you can only apply valHooks to nodeNames and input types, and not a collection of elements as before. We could theoretically privatize valHooks taking it off of jQuery and only use it internally for our own convenience, but again, I do not believe this patch carries with it the dangers of the first proposal.

- Slightly improved performance of val on radios and checkboxes for browsers that support checkOn, given the conditional attachment of its hook.
64a0005
@staabm

Missing var keyword?

Owner

Look up. Reusing an existing var. :)

@scottgonzalez
Member

I've only quickly glanced over this, but my objections to the previous proposal don't exist with this one :-)

@dmethvin
Member
dmethvin commented Apr 4, 2011

I like this!

@danheberden
Member

+1

@jeresig jeresig merged commit d47c0ae into jquery:master Apr 10, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment