Bug: angular.equals not symmetric given objects with non-enumerable properties

[qc00]

POC:

let o = {}; Object.defineProperty(o, 'a', {value:1});
angular.equals(o , {}); // true
angular.equals({}, o); // true
angular.equals(o, {a: 1}); // false
angular.equals({a: 1}, o); // true

What the results of these should be are debatable but the last two should be at least symmetric. (Presumbly, both should return false to retain transitivity.)

The use case of non-enumerable is I want to ignore certain deep/cyclical/non-important part of the tree from comparison when using Scope.$watch(..., ..., true).

[gkalpak]

(As mentioned several times) the angular.xyz helpers are not supposed to be general purpose utility functions that cover all use cases. In this case, the recommendation is to use a specialized library.

Is there some part of Angular that is using angular.equals that is causing you problems?

[qc00]

The use case of non-enumerable is I want to ignore certain deep/cyclical/non-important part of the tree from comparison when using Scope.$watch(..., ..., true).

I am not sure this feature can still be trusted if the underlying function behaves incorrectly in this use case.

[gkalpak]

So, are you trying to deep-watch an expression which sometimes points to an object with enumerable properties and sometimes to an object with non-enumerable? It sounds confusing.
Can you you post an example of what you are trying to do - e.g. a code sample that showcases the unexpected behavior?

[qc00]

There's no expected behaviour in the current implementation, but as the handling of non-enumerable properties are not guaranteed by the documentation of angular.copy, the adverse condition can arise in a future version.

Specifically, angular.copy's documentation says:

... and then all elements/properties from the source are copied to it.

In reality, non-enumerable properties are not copied right now.

If the implementation were to change to copy them, then the copies may not compare correctly with the original.

[wesleycho]

Currently the way to ignore certain parties of a deep tree is to pass a function that returns the expression to be watched, i.e.

$scope.$watch(() => $scope.foo.bar.baz, () => { ... });

I would be wary about changing Angular's core, as this would be a breaking change with potentially significant effects on some users' apps. IMO, this is a non-standard way to try to eke out perf benefits when there are already existing tricks.

[gkalpak]

@billccn, this sounds like a documentation issue. The behavior of copy is very specific, but the documentation could be lacking.

If it were to change in the future it would be a breaking change. So, this would only happen in a minor version update, be properly documented and would be expected to break things (that's what breaking changes do 😃).

But I don't think it is likely to ever happen.

If anyone is interested in updating the docs though, they would be more than welcome! 😃