Skip to content

Conversation

mjackson
Copy link

This commit makes the implementation of jQuery.fn.bind, jQuery.fn.one, and jQuery.fn.unbind more generic so that they may be called in the context of a jQuery-like object, as well as an actual jQuery object. This can be useful for 3rd parties who wish to use these functions to bind/unbind events for any set of arbitrary objects in an Array.

This commit genericizes the implementation of the core jQuery.fn event
bind/unbind functions so that they may be called in the context of any
jQuery-like object. This can be useful for 3rd parties who wish to use
jQuery's events subsystem for arbitrary objects in an Array.
@dmethvin
Copy link
Member

It's a pretty simple change, but I'd like to know more about how this is being used since it's implying that we will need to document and support this into the future.

@mjackson
Copy link
Author

The second commit (cd59836) demonstrates the typical use case for this style of implementation. In this case, instead of requiring another trip through the main jQuery function just to do a simple unbind, we can instead wrap the element in an array and pass that to jQuery.fn.unbind directly.

Ideally (and this is just pure speculation, not suggesting this should happen right now) all of the methods in jQuery.fn would be able to be used in the same way; calling them in the context of a simple array or a jQuery object shouldn't matter.

@dmethvin
Copy link
Member

I guess the question I should have asked was not "how" but "why". What is the need for this? If we put it in, how will it be used and how are we not satisfying that need today? Are you saying it's a performance optimization?

@mjackson
Copy link
Author

It's mainly an improvement in flexibility of the prototype (although there is one less function call as well). Right now, in order to effectively use any of the methods on jQuery.fn (the prototype) you need to instantiate a new jQuery object. This is demonstrated in the jQuery codebase in several places where jQuery() is used to wrap some arbitrary object, usually an element or a selector. However, in many of these instances a simple array will do. Except for the fact that most members of jQuery.fn assume that they're operating within the context of a jQuery object. This makes the jQuery.fn prototype a rather rigid one.

@dmethvin
Copy link
Member

Hmm, interesting idea. If there is no immediate need, I think it's worth considering in more detail and tackling all of this as an enhancement in a future release. Would you like to create a ticket and reference this pull request?

@@ -904,7 +904,7 @@ jQuery.each(["bind", "one"], function( i, name ) {
// Handle object literals
if ( typeof type === "object" ) {
for ( var key in type ) {
this[ name ](key, data, type[key], fn);
jQuery.fn[ name ].call( this, key, data, type[key] );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You've left out the event callback here. Also I'm not sure I see the advantage of switching to the generic prototype over using the prototype on this.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure that extra fn is actually a typo in the original source. After all, jQuery.fn.bind and jQuery.fn.one only take three arguments each. On that line, they're being called with four. Hence, it was omitted intentionally. And all tests pass.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, looking closer that would be for type/function key/value pairs and fn would not be used in that case.

@mjackson
Copy link
Author

@dmethvin I've opened a ticket to discuss this issue here.

@dmethvin
Copy link
Member

Per the ticket we've decided not to make these changes. If you'd like to advocate the issue further, make a post on forum.jquery.com in the Developing jQuery Core section.

@dmethvin dmethvin closed this Mar 30, 2011
@lock lock bot locked as resolved and limited conversation to collaborators Jan 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants