Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Conversation

@cexbrayat
Copy link
Member

Logs a warning in console when an angular expression contains an undefined key.
fixes #6204

This a naïve attempt to solve the problem in the related issue, but that's a first step to launch a discussion (if you have any interest in improving this).

I've added a simple $log.warn (is it a good idea? what level should that be? should we have some kind of options to enable or disable it?) when an expression has an undefined object in its path.

Even with this raw first implementation, it's doing a fine job on my projects and it's helping to find typos in expressions or unwanted null objects.

Exemple :

{{ person.name }} // person is undefined in scope
// will log in console
[$parse] Key `name` cannot be resolved in expression `person.name` because parent object is null

@mary-poppins
Copy link

Thanks for the PR! Please check the items below to help us merge this faster. See the contributing docs for more information.

  • Uses the issue template (#6414)

If you need to make changes to your pull request, you can update the commit with git commit --amend.
Then, update the pull request with git push -f.

Thanks again for your help!

src/ng/parse.js Outdated
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be better to inject $log and use $log.warn here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe. For now, the parser does not have access to $log, so I kept it that way.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually, @wesleycho is right here --- for browser compat reasons we can't really ask for window.console, this is going to cause problems. $log abstracts this nicely for us, use that please.

@cexbrayat
Copy link
Member Author

On the advice from @wesleycho and @caitp , it now uses $log to log the warning (in the generated getter and the simple ones). I also corrected the noise in the various tests : now we should have an assertion anywhere a test triggers a warning.

I'm looking forward to your feedback.

Logs a warning in console when an angular expression contains an undefined key.
fixes angular#6204
@IgorMinar
Copy link
Contributor

Hi @cexbrayat,

Thanks for the PR! This is a big change!

While I understand that forgiving expression evaluation has it's flaws, I don't think that logging every time a key can't be resolved is the right way to go. This forgiveness is more desirable than alternatives which would make you write if statements for any key that is optional.

Imagine having to use something like:

{{ user && user.name && user.name.first }}

in each binding that might not be always initialized or will be initialized lazily and asynchronously when the data arrives. That would be quite horrible.

We do have some ideas about how to deal with this though and will experiment them in Angular 2. For example, if we knew the "shape" of the scope using some kind of type information, we'd know what bindings are possible and error if we come across a binding that will never be fulfilled using the scope with the given shape. We'll see how that goes.

@cexbrayat
Copy link
Member Author

@IgorMinar Ok, thanks. I just want to point that I've been using this for some time on various projects, and it is not so verbose (usually, a simple $scope.user = {} in the controller solves it), and I was thinking of providing an option to disable it.

Anyway, I get your point and I'm looking forward to these experiments in Angular 2 :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Increase visibility of rendering issues

5 participants