Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#16667 closed defect (fixed)

[regression] dojo/_base/Deferred.when (via dojo/when) coerces to promise when an errback but no callback is passed

Reported by: Kenneth G. Franqueiro Owned by: Kenneth G. Franqueiro
Priority: undecided Milestone: 1.8.4
Component: Core Version: 1.8.3
Keywords: when Cc: Mark Wubben
Blocked By: Blocking:

Description

In Dojo 1.8, dojo/when has a feature (shared with other implementations) where simply passing an immediate value to it will yield a promise resolving to that value.

Unfortunately, due to how this feature is implemented, it leads to a bug, and a regression in dojo/_base/Deferred.when which now relies on dojo/when. Previously in 1.7, you could call Deferred.when(something, null, errback) to pass only an error handler for handling; if the value isn't a promise and no callback is provided, it would return the value straight through. If you do this in 1.8, however, it ends up wrapping the value in a promise, because of the code at https://github.com/dojo/dojo/blob/1.8.3/when.js#L39-43.

If we go about fixing this, also be wary of https://github.com/dojo/dojo/blob/1.8.3/when.js#L50 which currently assumes that if any of the 3 callbacks are defined, then we have a promise (which wouldn't necessarily be the case).

Attachments (1)

16667.diff (982 bytes) - added by Kenneth G. Franqueiro 6 years ago.
Proposed patch against 1.8 branch

Download all attachments as: .zip

Change History (7)

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

Cc: Mark Wubben added
Keywords: when added
Milestone: tbd1.8.4

Changed 6 years ago by Kenneth G. Franqueiro

Attachment: 16667.diff added

Proposed patch against 1.8 branch

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

I've attached a patch which I think resolves the issue without any side-effects. All when tests pass (including one I added to test this issue). Would like Mark to look at it if he gets a chance... it'd certainly be good to get this fixed for 1.9 as well.

comment:3 Changed 6 years ago by Mark Wubben

Owner: set to Kenneth G. Franqueiro
Status: newassigned

Patch looks good. Please merge it, I have no clue how to deal with SVN anymore.

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

Thanks Mark, I'll work on merging this into 1.8 branch and trunk later today.

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

Resolution: fixed
Status: assignedclosed

In [30605]:

Revise conditions under which dojo/when creates a promise; fixes #16667 in trunk !strict

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

In [30606]:

Revise conditions under which dojo/when creates a promise; fixes #16667 in 1.8 branch !strict

Note: See TracTickets for help on using tickets.