-
Notifications
You must be signed in to change notification settings - Fork 27.4k
Adds the ability to reregister a watcher thru the property restore
#13524
Conversation
b22b767
to
b5f540a
Compare
Add a new id property to the watches Use this id when locating this id to remove the watch
b5f540a
to
fa53655
Compare
The second commit message is wrong I believe. Restoring watchers is a nice-to-have functionality anf perf improvements are always welcome I guess, but I have some concerns:
I'm not sayng these are reasons not to move this forward, but rather things to take into consideration. I would be also interested in the motivation behind these changes ? Is there some specific usecase we are trying to support ? Did the need came up in a "real-world" app. tl;dr WRT the perf improvement:
WRT restoring deregistered watchers:
|
Even when performance improvement is tiny, but this change is needed for the second commit to be efficient.
I do not find anything magical in it. It can be simplified to
Thanks!
I would not call the implementation magical, it adds some small complexity with all the The need for this came from a real-world need to lower the number of watchers and restore them once some specific even happen (and not having to keep all the scopes/expressions/listeners nor having to mess with |
Add the function property `restore` to `scope.$watch`, `scope.$watchCollection` and `scope.$watchGroup` returning function. If the watcher was deregister, then calling the `restore` function restores the watcher in its original position. If the watcher is deregistered, then calling `restore` is a no-op.
fa53655
to
aaab4ca
Compare
I don't know if it's just me, but |
I like the idea to have a unique id every time a new watcher is created, in real life we sometimes need this (and so had implemented it our angular.js copy) to simplify debugging. |
We are planning to go with a more explicit version of this: #10658 |
Two commits: