Bug Tracker

Ticket #9937 (closed enhancement: wontfix)

Opened 3 years ago

Last modified 2 years ago

Deferreds, promises and jsXHR-s are plain objects.

Reported by: rkatic Owned by: rkatic
Priority: low Milestone: None
Component: ajax Version: 1.6.2
Keywords: Cc:
Blocking: Blocked by:

Description

Deferreds, promises and jsXHR-s, are created using the module pattern, producing plain objects.

jQuery.isPlainObject( jQuery.Deferred() ) === true;
jQuery.isPlainObject( jQuery.Deferred().promise() ) === true;
jQuery.isPlainObject( jQuery.ajax() ) === true;

This also means that on deep extending, those objects will be duplicated like plain objects and arrays.

Probably the easiest way to fix this, is to change a bit object/instance creation replacing obj = {} with obj = new jQuery.noop() and obj = {...} with obj = new jQery.extend({...}). In this way objects are not directly inherited from Object.prototype. Patch:  https://github.com/rkatic/jquery/compare/jquery:master...rkatic:no_plains.

Another possible problem is that instanceof do not work with deferreds.

( jQuery() instanceof jQuery ) === true;
( new jQuery.Event instanceof jQuery.Event ) === true;
( jQuery.Deferred() instanceof jQuery.Deferred ) === false;

To resolve this too, and to simplify true instance creations via module pattern, it could be useful to bring a utility method that in the following patch is called jQuery.alloc. Patch:  https://github.com/rkatic/jquery/compare/jquery:master...instanceof.

Change History

comment:1 Changed 3 years ago by dmethvin

  • Owner set to rkatic
  • Status changed from new to pending

I agree with the findings but don't see the need to change it. Are there some cases where the current implementation becomes a problem?

comment:2 Changed 3 years ago by rkatic

  • Status changed from pending to new

I suppose the problem can relatively easily arise on creating more sophisticated plugins, where usage of both deferreds/promises and jQuery.isPlainObject are combined. I suppose it's not too hard to imagine such thing (or I should provide an example?).

Regarding the instanceof issue, I suppose you have to decide if that statement should be supported or not.

comment:3 Changed 3 years ago by jaubourg

  • Type changed from bug to enhancement
  • Component changed from unfiled to ajax

You'll have no problem with Deferreds and Promises. The promise() method actually uses $.extend as a mean to attach the promise aspect onto a plain object (Deferreds and Promises are just collections of lexically-bound methods), so the behaviour is exactly as expected and changing it here will break stuff.

The jqXHR object is another story though, seeing as it has fields that are updated. So if you were to $.extend( {}, jqXHR ) before the request has completed, only the original object would get its status, statusCode and responseXXX fields updated. However, callbacks attached through the new object would still be given the original jqXHR.

So, using the extended jqXHR to retrieve fields would be a no-go but that's not really that big of an issue, given callbacks given to the extended object will receive the original jqXHR (and that's the object you should use to retrieve fields).

I dunno. Would the need be widespread enough to warrant some kind of "atomic object" registry that $.extend would look into to control if an object can be deep extended? I'm really no sure myself.

comment:4 Changed 3 years ago by rkatic

Deferred have no states exposed, jqXHR have -- so, as you pointed, former objects can be duplicated (deep extend) with no issues, but jqXHR can not.

However, even in case Deferred will never expose states in future, $.isPlainObject itself is part of the public API, and using it against those objects will give unexpected results.

the behaviour is exactly as expected and changing it here will break stuff

No-one is proposing to move methods to Deferred.prototype, if that was your concern.

Even if $.isPlainObject is mainly implemented by me, I always was against it because of similar issues. Now we have it in public API, and we have to deal with it. In case of Deferred, changing docs a bit would probably be enough (!?), but that would not be good enough for jqXHR anyway.

I dunno. Would the need be widespread enough to warrant some kind of "atomic object" registry that $.extend would look into to control if an object can be deep extended? I'm really no sure myself.

Screw the "atomic object" crap - at least, "instances" with exposed states should not be plain objects :)

Last edited 3 years ago by rkatic (previous) (diff)

comment:5 Changed 3 years ago by jaubourg

Deferred are not stateless... the state is not exposed that's all. Deferreds and Promises are plain objects and I'm fine with that: they've been purposedly designed that way.

comment:6 Changed 3 years ago by rkatic

Yea, for "state-less" I meant that states are not attached to object... I agree that such term was misleading (at least), so I fixed my previous post.

I am not sure if you are aware of what I am proposing. My first patch would fix the plain-object issue with minimal changes, with no breaks. Are you disposed to consider a similar change for jqXHR at least?

Last edited 3 years ago by rkatic (previous) (diff)

comment:7 Changed 3 years ago by addyosmani

  • Status changed from new to pending

Its a nice fix, but so far, this enhancement still doesn't appear to have a widespread use-case defined (that is generally agreed with) to warrant the change being made.

comment:8 Changed 3 years ago by rkatic

  • Status changed from pending to new

Excluding the instanceof issue, there are two problems:

1) $.isPlainObject( deferred/promise/jqXHR ) === true

If expected result is true, then ignore this, but if expected value is false, then in my opinion this is more a bug then an enhancement.

2) $.extend(true, ...) will break any copied jqXHR.

Definitely a bug. I don't get the argument that this is not an common use-case. Are we ignoring here that jQuery supports plugins and that such usage should work properly even if not present in the jQuery code itself?

Last edited 3 years ago by rkatic (previous) (diff)

comment:9 Changed 3 years ago by dmethvin

@rkatic, can you describe an application where extending a jqXHR is useful and could not be achieved by other reasonable means?

comment:10 Changed 3 years ago by dmethvin

  • Status changed from new to pending

comment:11 Changed 3 years ago by rkatic

  • Status changed from pending to new

@dmethvin: Maybe I was not clear. The point (2) is not about $.extend(true, {}, jqXHR), but about $.extend(true, {}, {xhr: jqXHR}).

So the point is not to make jqXHR safe on extending, but to prevent $.extend(true, ...) to duplicate contained jqXHR.

Last edited 3 years ago by rkatic (previous) (diff)

comment:12 Changed 3 years ago by timmywil

  • Priority changed from undecided to low
  • Status changed from new to closed
  • Resolution set to wontfix

Scoping is another commonly used tool. The fact that they are not instantiated has its own advantages. Not everything needs to conform to a standard practice of prototyping.

I think we're fine with them being plain objects and I don't see a common use case for the stated extend, even in plugins. Thank you for taking the time to work through this, but I don't think we'll be changing the implementation.

comment:13 Changed 2 years ago by katowulf@…

I ran into this problem today while trying to create a simple chain of promises. I wrap a series of functions which may or may not return a promise and execute them sequentially.

If the result is a promise, then execution of the next call in the chain is deferred until the promise resolves. Otherwise, the next method is called immediately. If an Error is returned, then the chain of execution is aborted.

Example in jsfiddle:  http://jsfiddle.net/katowulf/JCED9/1/

comment:14 Changed 2 years ago by anonymous

To clarify, there is no real method to determine if it's a Deferred or not (I could check 'deferred' in object, but could not another jquery object have a deferred property?

Without instanceof, I'm at a bit of a loss for an effective way to determine if something is a promise/deferred.

comment:15 follow-up: ↓ 16 Changed 2 years ago by rwaldron

A promise is defined as an object that has a function as the value for the property 'then'

 http://wiki.commonjs.org/wiki/Promises/A

comment:16 in reply to: ↑ 15 Changed 2 years ago by jaubourg

Replying to rwaldron:

A promise is defined as an object that has a function as the value for the property 'then'

 http://wiki.commonjs.org/wiki/Promises/A

That's the Promise/A way. You can also test for the presence of a promise method and call it to retrieve the promise... that way, you will also catch objects that do not implement the promise interface yet are observable through one (see $.when's code if you're curious).

Note: See TracTickets for help on using tickets.