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

fix($parse): unwrap promise when setting a field #3278

Merged
merged 1 commit into from
Jul 31, 2013

Conversation

chirayuk
Copy link
Contributor

This fixes an inconsistency where you can't call the setter function
when the expression resolves to a top level field name on a promise.

Setting a field on an unresolved promise will throw an exception. (This
shouldn't really happen in your template/js code and points to a
programming error.)

Closes #1827

@ghost ghost assigned IgorMinar Jul 18, 2013
@chirayuk
Copy link
Contributor Author

@IgorMinar@jeffbcross and I looked at the original PR #2854 and cleaned it up into this one.

@petebacondarwin
Copy link
Contributor

This just doesn't seem right to me. How can it be a programming error to try to write to a potentially unwrapped promise? Surely the point of unwrapping it automatically is that the programmer doesn't need to care whether the promise has been resolved or not? If they do care then they have to add their own then() handler to the promise to make sure it is resolved before they can write to it. In that case there is no benefit in unwrapping and the developer might as well just apply their own then() handler and copy the value onto the scope when the promise is resolved.

If you are going to do this then you ought to provide some caching mechanism that will store the written value until the promise resolves... But in my view the whole idea of auto unwrapping (even getters) is a bad one.

@chirayuk
Copy link
Contributor Author

@petebacondarwin – [1] removed the exception  [2] auto unwrapping promises is probably a bit too much magic. However, given that we're already doing it in the getter, I think we should at least be consistent about it when we do it on the setter.

@IgorMinar – I don't think throwing the exception is a good idea. I've updated the commit to make it equivalent to the way getters work and fixed some edge cases.

@chirayuk
Copy link
Contributor Author

This fixes an inconsistency where you can't call the setter function
when the expression resolves to a top level field name on a promise.

Setting a field on an unresolved promise will throw an exception.  (This
shouldn't really happen in your template/js code and points to a
programming error.)

Closes angular#1827
@chirayuk chirayuk merged commit 61906d3 into angular:master Jul 31, 2013
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug with asynchronous text field updates
3 participants