Opened 3 years ago

Closed 3 years ago

#18857 closed defect (wontfix)

dijit/Destroyable.destroy() breaks inheritance chain

Reported by: gerhard presser Owned by: bill
Priority: undecided Milestone: tbd
Component: Dijit Version: 1.10.2
Keywords: Cc:
Blocked By: Blocking:

Description

it doesn't call this.inherited(arguments).

if any (non) widget (e.g. dGrid), with an existing destroy() function, inherits from the module, the super-functions may not be called.

Change History (6)

comment:1 Changed 3 years ago by bill

Status: newpending

The idea is for Destroyable to be at the beginning of the inheritance chain. This is a basic principle of how OO programming works, that the class that initially defines a method doesn't call super() or this.inherited() etc. So I don't see why there should be an exception for the destroy() method. Or do you think (for example) that every method in dijit/WidgetBase should call this.inherited() just in case?

Probably in your case you need something like declare([Destroyable, DGrid], {}) to get Destroyable at the beginning of the inheritance chain. Does that work?

comment:2 Changed 3 years ago by gerhard presser

Status: pendingnew

yes, this would work, but isn't there a semantic difference?

declare([Destroyable, DGrid], {}) - a destroyable with dgrid mixed in
declare([DGrid, Destroyable], {}) - a dgrid with destroyable mixed in

is there any difference? do 'instanceof' calls still work?
is Destroyable realy the top-most place where destroy() gets declared?

comment:3 Changed 3 years ago by bill

is there any difference?

Sure, the difference is which class extends which, but AFAIK that difference won't be significant in this case.

do 'instanceof' calls still work?

Not sure. IIRC dojo/declare provides an .instanceOf() method that understands multiple inheritance (unlike the instanceof operator in javascript).

is Destroyable realy the top-most place where destroy() gets declared?

Yes, that's why the class is called Destroyable.

comment:4 Changed 3 years ago by dylan

In the case of dgrid, you generally don't need to have Destroyable... instead of this.own() to capture things to destroy, it will call things added with on, as well as things added via this._listeners.push() .

So are you only concerned about dgrid, or other non-widgets?

comment:5 Changed 3 years ago by gerhard presser

in this case just dgrid - yes maybe using _listeners would be an alternative. I just wanted to have one consistent way of handling resources.

the answers are OK for me - the ticket may be closed.

comment:6 Changed 3 years ago by bill

Resolution: wontfix
Status: newclosed

OK, thanks.

Note: See TracTickets for help on using tickets.