$watch/$eval expressions produce null unexpectedly

[spamdaemon]

Given this small fragment in a controller:

$scope.foo = null;
$scope.$watch("foo.bar",function(nv) {
   $log.info(nv);
});

produces 'null'

whereas

$scope.foo = {};
$scope.$watch("foo.bar",function(nv) {
   $log.info(nv);
});

produces 'undefined'

If I try to $eval the expressions I get 'null' and 'undefined', respectively, as well.

This seems to be somewhat inconsistent.
I'm using version 1.0.5

[chrisirhc]

This appears to be by design that it returns null or undefined if any of those values are reached first during evaluation of the expression.

Refer to the following lines:
https://github.com/angular/angular.js/blob/master/src/ng/parse.js#L773
https://github.com/angular/angular.js/blob/master/src/ng/parse.js#L848

I was about to post an issue about this. There is nothing about this in the documentation. If it is by design, there should be something in the documentation that mentions this.

[btford]

As part of our effort to clean out old issues, this issue is being automatically closed since it has been inactivite for over two months.

Please try the newest versions of Angular (1.0.8 and 1.2.0-rc.1), and if the issue persists, comment below so we can discuss it.

Thanks!

[IgorMinar]

reopening: I just came across this and I agree that it is an undesirable side-effect of how $parse works, we should fix it.

[IgorMinar]

@caitp do you have cycles to look into this one? the only tricky issue is that due to optimizations, promise unwrapping and csp mode there are several places that need to be touched.

we should always return undefined.

the test case should be something along these lines:

scope.obj1 = {};
scope.obj2 = null;
scope.obj3 = '';

expect(scope.$eval('obj0.bar')).toBeUndefined();
expect(scope.$eval('obj1.bar')).toBeUndefined();
expect(scope.$eval('obj2.bar')).toBeUndefined();
expect(scope.$eval('obj3.bar')).toBeUndefined();

[IgorMinar]

this should also fix #5442

[caitp]

I'll poke through it --- When exactly is the deadline for 1.2.6?

[IgorMinar]

I triaged it for 1.2.7. codefreeze for 1.2.6 is tomorrow noon-ish

[caitp]

http://plnkr.co/edit/01gNRN0noYfqOhmlAjNc?p=preview is a prototype of the changes, but it's probably missing some things if you have a few minutes to go over it --- I'm just going to start porting those changes into the code though.

I think the downside to this is it could potentially increase minified filesizes a fair bit, unless minifiers are clever enough to come up with an alias for "undefined"

...actually, I guess this should only return 'undefined' if things in the middle of the chain are missing, right? So it might be even worse :(

[IgorMinar]

I don't think that that's going to be a problem. Check out how the getter function is implemented (there are several versions).

check out

angular.js/src/ng/parse.js

Line 897 in bc3ff2c

if (!key1 || pathVal == null) return pathVal;

right now it returns pathVal, but it should return pathVal only if there is no more path key to be dereferenced (== when we are in the middle of a path)

[caitp]

Yeah, it's looking okay so far (without testing CSP mode or promise resolution, so unit tests will need to do those), but the problem (demonstrated at http://plnkr.co/edit/01gNRN0noYfqOhmlAjNc?p=preview) is with the simpleGetterFns --- basically you get the desired behaviour if you have enough keys to not use the optimized getters, otherwise you end up with a premature null.

This is very inconsistent, and I'm not too sure of a good way to fix that. (it would actually apply to any expression with 6 keys even, but hopefully those are rare enough)

[IgorMinar]

check out line 10331 in http://plnkr.co/edit/hE1UEGBqCTGKXlssTaYw?p=preview

I changed:

return scope == null ? scope : scope[key1];

to be

return scope == null ? undefined : scope[key1];

that obviously fixes this issue only for paths with 2 segments and only when promise unwrapping remains disabled.

but it should give you an idea of how to go about fixing this.

[caitp]

I wrote a suite of (very meta-programmed unit tests, hope that's okay) and they all seem to be passing (but due to the heavy meta-programming, careful review and comments are needed) --- These aren't setting up CSP or enabling promise unwrapping, though, so a bit more meta stuff is needed... Just looking to find all of the related issues before pushing. I guess it's just these 2.

[IgorMinar]

This was fixed by #5480 which landed yesterday