Opened 9 years ago

Closed 8 years ago

Last modified 7 years ago

#12429 closed enhancement (fixed)

Split out connect(), subscribe() from dijit._WidgetBase into a separate mixin

Reported by: Colin Snover Owned by: bill
Priority: high Milestone: 1.8
Component: Dijit Version: 1.6.0
Keywords: Cc:
Blocked By: Blocking:

Description (last modified by bill)

I often want to have a simple destroyable object that tracks its own connects and subscriptions that isn’t necessarily a widget with a DOMNode. I’d like to pull out connect, disconnect, subscribe, unsubscribe, and the relevant parts of destroy and destroyRecursive from _WidgetBase and put them into a different subclass—perhaps one that isn’t even part of dijit, if that makes sense (kind of like dojo.Stateful; dojo.Connectable?).

I asked phiggins about this and he mentioned he often creates a dijit._Midget subclass to do this on his own projects as well so I imagine it is a use case a good number of people run into at some point.

Attachments (2)

trackHandle.patch (2.0 KB) - added by bill 8 years ago.
untested patch for trackHandle() helper method
Attributed.js (4.3 KB) - added by Kitson Kelly 8 years ago.
A class the support auto-magic getters and setters

Download all attachments as: .zip

Change History (13)

comment:1 Changed 9 years ago by bill

Description: modified (diff)
Summary: Split out self-scoping and cleanup stuff from dijit._WidgetBase into a separate classSplit out connect(), subscribe() from dijit._WidgetBase into a separate mixin

I can see how that would be useful, although before creating a new class should meditate on the API and names. connect() and subscribe() were bad choices for names because they sounds like external API's. IOW, users would expect this to work but it doesn't:

var foo = new MyWidget();
foo.connect("onClick", anotherClass, "methodInAnotherClass");

Maybe the functions should be overloaded to support that kind of usage too?

comment:2 Changed 8 years ago by bill

New developments:

  • dojo.connect() is being replaced by dojo/on and dojo/aspect. dojo.subscribe() is being replaced by dojo/topic.
  • dojo.disconnect() and dojo.unsubscribe() are deprecated, new way is to just call handle.remove(). Therefore, calling handle.remove() on a handle returned by protected methods like _Widget.connect() should remove the handle from this._connects[].
  • _WidgetBase has a public method called on() used for app code monitoring events on the widgets, rather than by widget code monitoring events on DOMNodes or other objects. Any protected methods would need to have a different names.

One option is to have a method called trackHandle() that could be used like this:

declare([Connectable, on, aspect, topic], {
    foo: function(){
          ...
          this.handle1 = this.trackHandle(on(domNode, "click", ...));
          this.trackHandle(aspect.after(myObj, "foo", ...));
          this.trackHandle(topic.on("/dnd/start", ...));
   },
   bar: function(){
          // just calling handle1.remove() will deregister it from this._connects
          this.handle1.remove();
   }
});

Not sure if that's good or not. Admittedly it's more cumbersome to use that the current _Widget.connect() and _Widget.subscribe().

I'll attach the patch though.

Last edited 8 years ago by bill (previous) (diff)

Changed 8 years ago by bill

Attachment: trackHandle.patch added

untested patch for trackHandle() helper method

comment:3 Changed 8 years ago by bill

See also #14224, about watch().

Changed 8 years ago by Kitson Kelly

Attachment: Attributed.js added

A class the support auto-magic getters and setters

comment:4 Changed 8 years ago by Kitson Kelly

I have added Attributed, a class that provides custom getters and setters that build upon dojo/Evented and dojo/Stateful classes that I think meets the intention of this ticket.

comment:5 Changed 8 years ago by bill

Seems like Attributed is something different from this ticket's intention, since Attributed is about get/set/watch and this is about connect/subscribe handle tracking. (I know Colin referenced this ticket from the Attributed email thread, but not sure why.)

Also, wanted to mention [28120] which is similar to the trackHandle patch I attached above.

Last edited 8 years ago by bill (previous) (diff)

comment:6 Changed 8 years ago by bill

So the two approaches are either having a general adopt() method:

declare([Connectable, aspect, topic], {
    foo: function(){
          ...
          this.adoptHandles(
              someObject.on("foo", ...),
              aspect.after(anotherObj, "foo", ...),
              topic.subscribe("/dnd/start", ...),
              thirdObj.watch("bar", ...)
          );
   }
   ...
});

Or having specific methods for each possible handle-generating action:

declare([Connectable], {
    foo: function(){
          ...
          this.adoptOn(someObject, "foo", ...);
          this.adoptAspect.after(anotherObj, "foo", ...);
          this.adoptSubscribe("/dnd/start", ...);
          this.adoptWatch(thirdObj, "bar", ...));
   }
   ...
});

I'm still leaning towards the first one since it seems more extensible. It could handle, for example, dojox.cometd.subscribe().

Last edited 8 years ago by bill (previous) (diff)

comment:7 Changed 8 years ago by bill

Milestone: tbd1.8
Owner: set to bill
Status: newassigned

comment:8 Changed 8 years ago by bill

I'm going to check in a class for this. Probably, it should be in dojo/, but putting it into dijit/ for the time being.

comment:9 Changed 8 years ago by bill

Resolution: fixed
Status: assignedclosed

In [28560]:

Make Destroyable mixin to track handles or other objects (including supporting widgets) owned by the instance, and to remove/destroy those objects when the instance is destroyed.

Also made _WidgetBase.destroy() automatically destroy all supporting widgets, without needing to register them via this._supportingWidgets[] or this.own(), unless they are special widgets that relocate themselves outside of this.domNode (i.e. popup widgets like Tooltip or Dialog).

Code partly from [28120], fixes #12429 !strict.

comment:11 Changed 7 years ago by bill

In [29339]:

add deprecation warnings to summaries, refs #12429 !strict.

Note: See TracTickets for help on using tickets.