feat($rootScope): Add custom compare and copy functions to $watch
Closes #10069
I'm sorry, but I wasn't able to verify your Contributor License Agreement (CLA) signature. CLA signature is required for any code contributions to AngularJS.
Please sign our CLA and ensure that the CLA signature email address and the email address in this PR's commits match.
If you signed the CLA as a corporation, please let us know the company's name.
Thanks a bunch!
PS: If you signed the CLA in the past then most likely the email addresses don't match. Please sign the CLA again or update the email address in the commit of this PR. PS2: If you are a Googler, please sign the CLA as well to simplify the CLA verification process.
CLA Should be OK now.
It seems to work well for my use case (angular-gantt). I've created a custom watcher with equals and copier supporting momentJS properly through lodash. See https://github.com/angular-gantt/angular-gantt/blob/history/src/plugins/history.js#L24-L53.
CLA signature verified! Thank you!
Someone from the team will now triage your PR and it will be processed based on the determined priority (doc updates and fixes with tests are prioritized over other changes).
CLA signature verified! Thank you!
Someone from the team will now triage your PR and it will be processed based on the determined priority (doc updates and fixes with tests are prioritized over other changes).
CLA signature verified! Thank you!
Someone from the team will now triage your PR and it will be processed based on the determined priority (doc updates and fixes with tests are prioritized over other changes).
Is there anything to do to see this getting merged ?
I cannot speak for the rest of the team, but there is something that make me unease about this PR. The digest cycle is a critical part of the code, and every single change has consequences. This is why there is a strict control to know when code from angular is executed and when code outside angular is executed, and be able to handle them with the right care. In this specific case, there are two points:
- we are 100% sure that
equalsandcopyhave no side-effects - we can control that
equals(instance, copy(instance)
If this PR is merged, then these assumptions are no longer true. I am specially worried about the first of these points
The PR also makes other small mistakes as it exposes the watch object as this to the copy and equals functions, but those are small nits that are easy to fix
The more I think about this, I think that most of what this PR brings can be done within a third party module. It would be something like
/* Warning, untested code, and can use some cleanup */
angular.module('foo', []).service('watchHelper', function($parse) {
return function(expression, customEquals, customCopy, listener) {
var previousPreviousValue;
var previousValue;
var firstCall = true;
var counter = 0;
expression = $parse(expression);
return {
expression: function(scope, locals) {
var newValue = expression(scope, locals);
if (firstCall && !customEquals(newValue, previousValue)) {
counter++;
previousPreviousValue = previousValue;
previousValue = customCopy(newValue);
}
return counter;
},
listener: function(newValue, oldValue) {
if (newValue === oldValue) {
listener(previousValue, previousValue)
} else {
listener(previousValue, previousPreviousValue)
}
}
};
};
});
// Then, in your code you do
var foo = watchHelper(expression, customEquals, customCopy, listener);
$scope.$watch(foo.expression, foo.listener);
@Narretz and I discussed a use for this functionality today inside ngModel.
I would like us to make a decision about whether we should include this:
- It seems to me that the normal use cases are untouched by this PR, which means that unless you decide to provide your own custom equals and copy the watching would be the same in terms of semantic and performance.
- While we would have no control over whether the custom
equalsandcopymethods had side effects and were fast, this is not really any different than concerns over the user providedwatcherfunctions. - Although this can be done using a helper, as suggested by @lgalfaso, this would be less performant than if it was embedded in the core root scope, and is more fiddly to set up.
- I think we should ensure that a custom
equalsmust be accompanied by a customcopyfunction.
TODO:
- [ ] Update watch delegates with this functionality too
- [ ] Check that both equals and copy are provided
- [ ] Add better docs
- [ ] Add bench press tests