Bug Tracker

Ticket #7818 (closed bug: fixed)

Opened 4 years ago

Last modified 3 years ago

META: Define validity of plain JS objects for jQuery() and its methods

Reported by: dmethvin Owned by:
Priority: low Milestone: 1.6.1
Component: core Version: 1.4.4
Keywords: Cc:
Blocking: Blocked by:

Description (last modified by ajpiano) (diff)

In the past a jQuery collection has mainly been DOM elements, with document or window allowed in some cases. We are starting to encourage wrapping plain JS objects in jQuery for purposes like pub/sub, and also seeing tickets like #7500 where people expect to use .attr() on plain JS objects.

We need to document the extent to which we support JS objects and decide the behavior of methods that don't accept JS objects. For example, $(obj).removeAttr("x") when used on a JS object currently sets the property to an empty string, it seems like the correct semantic action is to delete obj.x instead. BUT ... is it worth taking the size/performance/complexity hit to orthogonally handle something that is relatively rare?

PS, How does .data() differ from attr() on a plain object?

PPS, TEST CASES.

Change History

comment:1 Changed 4 years ago by dmethvin

The jquery-datalink plugin depends on the use of $(jsobj).data() and the data events it fires in order to do its synchronization with a form.

comment:2 Changed 4 years ago by snover

  • Priority changed from undecided to blocker
  • Status changed from new to open
  • Component changed from unfiled to core
  • Milestone changed from 1.next to 1.5

.data really doesn’t make much sense in the context of a non-jQuery object at this point since it just ends up pointing the data cache to the object itself. I don’t feel like this is necessarily the right thing to do (it seems to me that it’s proper to let JS objects have metadata too instead of just hooking directly onto the object in a way that is likely to collide), but it is what happens currently, and I’m not sure I have the gumption to deal with it.

.attr will break completely on JS objects in 1.5 without a compatibility sim because it is being changed to always only set attributes via the DOM attribute manipulation methods, not properties or attributes. I have no idea what .val does currently.

.bind/.trigger are used for custom event dispatching, even though JS objects can’t bubble, so most of the event system just ends up being overhead. .delegate/.live makes no sense.

.animate works on JS objects as well as DOM elements, and just cycles properties. .queue and friends needs to work.

.serialize documentation should probably have a link to jQuery.param, since that is the appropriate way to serialize objects, but otherwise should not function on JS objects.

Since we currently perform pub/sub, etc. using $(jsObj), it seems like that paradigm will need to stick around for a long time, even though it doesn’t make a lot of sense.

comment:3 Changed 4 years ago by paul.irish

.data() i have used on plain objects. specifically XHR objects. i'd like to retain that.

.attr() and .removeAttr() i would expect to not do anything to plain objects. i don't think jquery core should tackle it.

.bind() and trigger() should work on plain objects so people have a way of rolling their own pub/sub until one is provided.

.animate() on a plain object was explored by james padolsey and should continue to work.

comment:4 Changed 4 years ago by seankoole

to give some more perspective on what the current situation is

attr:  http://jsfiddle.net/Gs8LU/1/

  • attr (get), yes
  • attr (set), no
  • removeAttr, no

data:  http://jsfiddle.net/Gs8LU/

  • also animate and bind

If you choose to let data store $.fn.data outside of the element, then $.fn.attr should support plain objects.. it would make no sense to create a suggest $.fn.prop just for this, I'd rather see it in $.fn.attr and document it properly.

If the choice is to keep data the same way as is, then $.fn.attr needs to be only for dom elements.

Like I said, I like the first option as it normalizes everything to how the dom works.

Last edited 4 years ago by seankoole (previous) (diff)

comment:5 follow-up: ↓ 6 Changed 4 years ago by snover

@paul_irish: As of 1.4.3, calling .data on anything (like an XHR) just attaches properties directly to the object, so there is no benefit to doing that vs simply setting the property directly. Furthermore, if you were doing that in 1.4.2, you were leaking memory every time you had an XHR.

comment:6 in reply to: ↑ 5 Changed 4 years ago by john

Replying to snover: "As of 1.4.3, calling .data on anything (like an XHR) just attaches properties directly to the object, so there is no benefit to doing that vs simply setting the property directly." Sure there is - it triggers events if they're bound - like in the case of the official jQuery plugin "Data Link".

 http://api.jquery.com/category/plugins/data-link/

Making this change (which was intentionally introduced) as to make this sort of functionality possible would be a MAJOR change and break a lot of code.

comment:8 Changed 4 years ago by scott.gonzalez

I'm not sure if this is expected behavior or not, but .trigger() will execute functions of the same name as the event. This can be handy because you can use event.preventDefault() to prevent the method from executing, or this can be confusing because you're not expecting it. I think the current implementation is fine, since it can be a nice feature and can be avoided by using .triggerHandler().

 http://jsbin.com/enahu4/edit

comment:9 Changed 4 years ago by john

  • Milestone changed from 1.5 to 1.6

We should look into this for 1.6.

comment:10 Changed 4 years ago by TrippingTheBits

There are already jQuery.data( element, ... ) functions. I'm curious what people think about having jQuery.data( plainObject, ... ) and jQuery.bind( plainObject, eventType, ... ) functions instead of allowing jQuery to wrap plain objects.

comment:11 Changed 4 years ago by timmywil

attr no longer works on plain objects in the attrhooks updates, since we are mainly using get/setAttribute.

Last edited 4 years ago by timmywil (previous) (diff)

comment:12 Changed 4 years ago by timmywil

However, the new jQuery.fn.prop AND jQuery.fn.removeProp work for getting, setting, removing for plain objects.

comment:13 Changed 4 years ago by john

  • Milestone changed from 1.6 to 1.next

comment:14 Changed 4 years ago by john

  • Keywords 1.7-discuss added

Nominating ticket for 1.7 discussion.

comment:15 Changed 4 years ago by rwaldron

  • Description modified (diff)

+0,

comment:16 Changed 4 years ago by jaubourg

-1, I'm not sure we encourage wrapping plain objects. We rather see people making use of it in some creative ways (pub/sub).

comment:17 Changed 4 years ago by timmywil

+0, Work needs to be done to figure out what doesn't already work and see if it would actually take much work. FYI, prefer prop over attr.

comment:18 Changed 4 years ago by dmethvin

  • Description modified (diff)

+1, I still don't think we have this under control, I would prefer to see us use $() for DOM objects only. Is the hack we put in for the now-unsupported .datalink plugin the way we want to do that long term?

comment:19 Changed 3 years ago by john

  • Description modified (diff)

+1, I think we pretty much took care of this in 1.6 with the separation of attr and prop.

comment:20 Changed 3 years ago by ajpiano

  • Description modified (diff)

+0, So what would actually have to happen here, Dave, John - Guess we'll find out in the meeting

comment:21 Changed 3 years ago by rwaldron

  • Keywords needsdocs added; 1.7-discuss removed
  • Priority changed from blocker to low
  • Milestone changed from 1.next to 1.6.1

comment:22 Changed 3 years ago by timmywil

  • Milestone changed from 1.6.1 to 1.next

comment:23 Changed 3 years ago by rwaldron

  • Milestone changed from 1.next to 1.6.1

The milestone was set to 1.6.1 to reflect the resolution from yesterday's meeting.

comment:24 Changed 3 years ago by timmywil

can this ticket be closed?

comment:25 Changed 3 years ago by timmywil

  • Status changed from open to closed
  • Resolution set to fixed

comment:26 Changed 3 years ago by dmethvin

To clarify the docs, the only operations allowed on plain objects after they are wrapped in jQuery() are:

.data()
.prop()
.bind()
.unbind()
.trigger()
.triggerHandler()

Note that using .data() -- or any method that requires .data() such as .bind() -- on a plain object will create a jQuery123456789 (random number) property on the object.

If .trigger("eventName") is used, it will look for a "eventName" property on the object and attempt to execute it after any attached jQuery handlers are executed. (Note that it does *not* check to see if the property represents a function.) To avoid this behavior, use .triggerHandler("eventName").

comment:27 Changed 3 years ago by dmethvin

  • Keywords needsdocs removed
Note: See TracTickets for help on using tickets.