Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#15364 closed defect (fixed)

dojo/Stateful watch handle API violates "remove" convention

Reported by: Colin Snover Owned by: Kris Zyp
Priority: blocker Milestone: 1.8
Component: General Version: 1.7.2
Keywords: Cc: cjolif
Blocked By: Blocking:


The handle returned by dojo/ has an "unwatch" method. It should have a "remove" method to match all other handles.

Change History (5)

comment:1 Changed 10 years ago by bill

Presumably (as mentioned in #10839) that's to match the Mozilla API, but it would be nice to be consistent with other handles, especially for the sake of Destroyable. unwatch() could also remain in 1.x, for back-compat.

Apparently Stateful doesn't really match the Mozilla API anyway, because in Mozilla unwatch() is a method on the Object, not on a handle returned from watch().

PS: here's the the painful email thread about naming.

PPS: this ticket is certainly not a blocker; it's not even a defect.

comment:2 Changed 10 years ago by Colin Snover

I’d expect both to exist in 1.x, maybe in 2.0 too if unwatch is a standard name.

I marked this as blocker now because it is a tiny thing from a code perspective (one-line fix) but is really important for being able to market DTK to engineers so I don’t want to see it slip 1.8 freeze. If I say “Dojo is great to use because it has really consistent API conventions, for example all handles have a remove function” and two (or more) core components are violating that convention, it hurts the toolkit’s image. If you want to change away from defect that’s fine but I consider this a consistency violation which would seem to me to be a defect.

comment:3 Changed 10 years ago by cjolif

Cc: cjolif added

comment:4 Changed 10 years ago by Colin Snover

Resolution: fixed
Status: newclosed

In [28714]:

Add 'remove' method to return handle to match handles from other methods, and fix the remove function so it does not destroy other handles when called more than once. Fixes #15364, #13282.

comment:5 Changed 10 years ago by bill

In [28727]:

Call remove() rather than unwatch(), since remove() has been added to the handles returned from watch(), refs #15364 !strict.

Note: See TracTickets for help on using tickets.