Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#16721 closed enhancement (worksforme)

dojo/Deferred do not resolve promise in the same event loop

Reported by: Kitson Kelly Owned by: Mark Wubben
Priority: undecided Milestone: 1.9
Component: Core Version: 1.8.3
Keywords: Cc: Kenneth G. Franqueiro, Colin Snover, Eugene Lazutkin
Blocked By: Blocking:

Description

According to the Promise A+ Spec, resolution (or rejection) of promises should not occur in the same turn of the event loop (item 4.1).

By not doing this, you can run into issues large chains of promises that are already resolved, making the call stack unmanageable.

The attached patch resolves this issue.

Attachments (1)

Deferred.patch (870 bytes) - added by Kitson Kelly 6 years ago.
Resolves/rejects promises asynchronously.

Download all attachments as: .zip

Change History (7)

Changed 6 years ago by Kitson Kelly

Attachment: Deferred.patch added

Resolves/rejects promises asynchronously.

comment:1 Changed 6 years ago by Kitson Kelly

Cc: Kenneth G. Franqueiro Colin Snover added
Milestone: tbd1.9
Owner: set to Mark Wubben
Status: newassigned

comment:2 Changed 6 years ago by bill

Cc: Eugene Lazutkin added

I'd be against this change. Besides slowing down performance, it will cause problems with various code, for example the parser, which assumes that promise.then(func) runs synchronously if the promise is already resolved.

There's also an issue with debuggability. Any code with setTimeout() is hard to debug, because it doesn’t have any call stack history. For truly asynchronous cases like when waiting for the server to process an XHR, this is unavoidable, but I don't want to make things worse than necessary.

Actually, the Deferred code used to work asynchronously like this, and Mark changed it after Eugene talked to him about the above issues.

comment:3 Changed 6 years ago by Kitson Kelly

Well, then we won't have a Promises A+ compliant solution.

My biggest issue is that you end up with ridiculous call stacks, with recursive sorts of promise chains (like the addition of the constructor in the parser returning a deferred, where you bill had to patch the parser back to not use promises).

The biggest issue I have though is that when when is passed a value, it does not invoke the callback immediately, it invokes it through creating another Deferred, and then in you end up with these complex promise chain callstacks.

It maybe not possible with 1.X, but we need/should provide a compliant solution in 2.0 or change the standard.

comment:4 Changed 6 years ago by Mark Wubben

We should be Promises/A+ compatible in 2.0. Synchronous inspection is being specified (<https://github.com/promises-aplus/synchronous-inspection-spec/issues/6>) which, in critical parts of the library code, would let us treat a fulfilled-promise the same as an actual value. We can't do this in 1.x for compatibility reasons.

I don't think there's much sense in having this ticket open for 2.0, so OK if I close it?

comment:5 Changed 6 years ago by Kitson Kelly

Resolution: worksforme
Status: assignedclosed

Yup, agreed.

comment:6 Changed 6 years ago by Kenneth G. Franqueiro

I realize this has already been closed for the time being, and I generally agree that this should be a 2.0 thing if even that.

My 2c though: I'd rather have long "hard to manage" chains that actually eventually trace back to what cause an error, than chains that don't actually trace back to the cause to begin with - and detaching things into the next turn causes the latter to happen more often than not. I'd want us to be reeeeally careful about implementing this for 2.0 so that we don't make debugging a nightmare in the opposite direction - asynchronous debugging of Deferreds has already frustrated me enough in the past.

FWIW, I believe in the past Colin pointed out that Q (https://github.com/kriskowal/q) may have had an interesting approach to debugging async promises.

Note: See TracTickets for help on using tickets.