Opened 8 years ago

Closed 8 years ago

Last modified 8 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:

Description

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

Change History (5)

comment:1 Changed 8 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 8 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 8 years ago by cjolif

Cc: cjolif added

comment:4 Changed 8 years ago by Colin Snover

Resolution: fixed
Status: newclosed

In [28714]:

Add 'remove' method to Stateful.watch 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 8 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.