Tree: exceptions to console when expand/collapse animation interrupted

[jonferraiolo]

See discussion in this Maqetta issue: ​https://github.com/maqetta/maqetta/issues/3519

The Maqetta team believes that dijit.Tree is not handling deferred properly.

[bill]

This error, or a similar one, can be reproduced by quickly expanding and collapsing a TreeNode (for example, the root node in the first tree in test_Tree.html).

Tree uses Deferred's to keep track of the expand/collapse animation of TreeNodes. The animations are cancelled (like in the above case) by canceling the Deferred. Canceling a Deferred counts as rejecting it, and by design, in debug mode, whenever a Deferred is rejected, and the rejection is not handled, there's a log message to the console.

The Tree code internally has some cases where it is not catching rejections on the Deferred objects returned from TreeNode.expand() and TreeNode.collapse().

In your case, you are calling selectPath() which ends up calling TreeNode.expand() which calls this._collapseDeferred.cancel(). this._collapseDeferred is the Deferred returned from collapse(). It's silly because the collapse animation has presumably already finished, but anyway that's enough to trigger the errors.

[bill]

In [30124]:

Refactor Tree code using Deferreds, leveraging chaining abilities of new dojo/Deferred module that allow expressions like promise1.then(function(){ return promise2; }).then(function(){ return promise3; }), which don't resolve until promise1, promise2, and promise3 have all resolved.

This indirectly fixes spurious console error messages when a Tree animation is cancelled, because for example the user started expanding a node but then clicked again to collapse it, before the original animation was finished. The problem was that there was previously code like animationDeferred.then(successHandler), with no failureHandler, and that caused dojo/promise/instrumentation to print the log messages.

Fixes #16430 !strict.

[bill]

In [30125]:

Return Promise rather than Deferred from public methods, but as usual shimmed with the addCallback() and addErrback() methods for backwards compatibility.

Refs #16430 !strict.

[ben hockey]

did you check that the shimming in r30125 actually works? as far as i know, the promise is frozen (so you'll need to check this in a browser that supports Object.freeze) and you can't add methods to it. the way to work around it is to use lang.delegate and then add the properties to the delegated object.

i don't know that what you've done won't work but it looks like it may not.

[bill]

I tried in Chrome, with no exceptions being thrown, but on further testing it appears to just fail silently. I guess I'll try adding a lang.delegate() although not sure if that works either, as it creates a new this pointer.

[bill]

PS: delegate() seems to work fine.

require(["dojo/Deferred", "dojo/_base/lang"],
 function(Deferred, lang){
   var d = new Deferred();
   var p = d.promise;
   var pd = lang.delegate(p, {
      addCallback: function(cb){ p.then(cb); },
      addErrback: function(eb){ p.otherwise(eb); }
   });
   pd.then(function(val){ 
      console.log("then returns ", val);
   }, function(err){
      console.log("then exception ", err);
   });
   pd.addCallback(function(val){ 
      console.log("addCallback returns ", val);
   });
   pd.addErrback(function(err){ 
      console.log("addErrback returns ", err);
   });
   d.resolve(43);
   //d.reject(new Error("testing"));
});

[bill]

In [30131]:

Need to use lang.delegate() to add methods to Promise because it might be frozen, thanks neonstalwart. Refs #16430 !strict.