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

Keep option to unwrap promises in template #5153

Closed
lyschoening opened this issue Nov 26, 2013 · 10 comments
Closed

Keep option to unwrap promises in template #5153

lyschoening opened this issue Nov 26, 2013 · 10 comments

Comments

@lyschoening
Copy link

This issue regards the previously closed issues #4158 and #4270. As I understand, unwrapping promises is now deprecated and on track for quick removal since unwrapPromises is clearly not intended to be much more than a helper for upgrading.

This recent change affects much of the code base of my application, which is heavy on nested promises that cannot be easily resolved with an unwrap() function as suggested in #4158. I think the change breaks AngularJS for many use cases that were not considered so far. I very much hope that a workable option for automatic unwrapping is (re)introduced before the deprecation becomes final.

While resolving one level of promises in the controller is already quite cumbersome (via e.g. promiseX.then (x) $scope.x = x for one item), it becomes impossibly complex with patterns as you might use with hierarchical or graph data structures:

  • Nested promises
  • Lazy loading via Object.defineProperty
  • Function calls in the template

Whenever you have nested promises, you either need to block while you resolve all promises; or have controllers with every single ng-repeat that do nothing more than resolve promises and copy them into the scope (e.g. ($scope) -> $scope.a.b.then (b) -> $scope.aB = b). As far as I know, AngularJS doesn't even provide a good helper function for resolving nested promises.

An unwrap() function is no solution, because you can't write something like unwrap(item).name in the template. You have to write a controller that blocks the expression from rendering until the promise is resolved, or at the very least insert ng-if="unwrap(promise)" on those tags that make use of a promise. On the other hand, it would have been very possible to write a wrap(promise) function that prevents a promise from resolving, if preventing automatic resolution was the objective of #4158.

I very much hope that a workable option for automatic unwrapping is (re)introduced before the deprecation becomes final.

@wizardwerdna
Copy link
Contributor

Can you provide a gist or plunkr describing the usecases you consider unworkable without it? I'm not really sure what you mean by "nested promises" or "function calls in the template," and how they interact with the change.

In my view, I think the team got it right. Burying promises in a magical "unwrapping," seems to be both too much and not enough. The promise object has so much it can do, either by aggregation (is that what you meant by nested) or by chaining .then functions, and almost all that value is lost by the unwrapper.

Frankly, I find the pattern:

   codeForPreResolutionState();
   get-data-promise(queryparameters).then(function(value){
     codeForPostFirstResolutionState(value);
     if (badness) { throw(new Error('badness happened); }
     return nextPromiseAfter(value)
   }).then(function(value){
     codeForPostSecondResolutionState(value);
   ....
   }).then(undefined, function(reason){
     codeForFailureState(reason)
   })

much, much cleaner than most alternatives for the use cases I imagine. The virtues of being able to throw from that logic with reasons and rebuilding promises for the next prong makes asynchronous code as easy as try-catch.

And having them in the controller/view-model seems a better way to handle the presentation state. The problem I always had with wrapping in the view was too much logic and hidden logic in the view, with the defaults being too much and not enough. I'm not sure that there is anything you can't do better in the controller, and you can always write your own "my-promise-bind" directive, patterned after the ng-bind directive.

If they wanted to keep it around, perhaps it would be best to do it with an ng-promise-bind directive.

@lyschoening
Copy link
Author

By nested promises I mean promises that resolve into objects that have attributes which are themselves promises. This makes a lot of sense with resources from RESTful services that contain cross-references to other resources in the form of URLs. I know ngResource has no support for this sort of complexity, but I am working with a REST library that does.

I've posted an example over here of what used to be possible with AngularJS 1.2.0: http://plnkr.co/bWBcdtfDaEVnJ2rmbgXV

In the example a collection of books is loaded. Each book references an author, which is a separate resource, and editions, a separate collection that is lazily loaded on demand. The two nested fields are specified when making the initial call.

@wizardwerdna
Copy link
Contributor

@lyschoening Lars, as I understand your definition "nested promises" are managed by the mechanics of the promise resolver itself. A promises-aplus standard compliant promises library will always resolve a promise for a value that is in turn a promise recursively, until rejected or resolved with a non-promise value. (Resolution procedure 3(b)). Promises cannot "contain" a promise, which is why they are not actually monads. When

p.then(function(value){...})

is established, onFulfilled should never be called with value equal to a promise. There are some edge cases, because the standard allows non-native promises to be values. Still, as you can see from the standard, almost any "thennable," that is an object with property then, will be "resolved" to its underlying value. The standard goes through great pains to preclude nested promise use with non-standard promises using, for example modern object getters and the like.

Of course the handler can itself return a promise, throw an error or the like, as part of promise chains such as I discussed in my prior message.

NB: The standard also tries to prevent self-resolving promises to avoid looping during promise resolution. Again, pains are made to avoid abuses using non-standard thenables, but Rice's Theorem suggests you can only do so much to capture those scenarios. Note that jQuery deferred's are not standards-compliant, if that is serving as the basis of your understanding. $q is standards compliant (in a sense, it's interaction with the $digest loop as its 'nextTick' means that you need to test it with a separate 'ticker' to obtain compliance) to the previous draft of the standard, and I am working to bring it up to version.

@wizardwerdna
Copy link
Contributor

@lyschoening

Here's the thing. A promise is not the same as its value, and it does not change into its value upon resolution. It is a present representation of a present or future value, to be resolved solely by registering handlers. The pressure put on $q resulting from the promise resolution (such as a property indicating whether the promise is presently resolved) has led to uncleanliness and standards noncompliance. I think the team made a decent choice backing it out of the core mechanics.

Thanks very much for the plunker, Lars. It focuses and makes clear what you are lamenting over, and how to address these issues. Your example is an app that loads an object with what we would call "associations" in rails and other ORMs, and you make provisions to load associations directly (when the main is loaded) or lazily (by tossing logic into the property accessor).

Nothing in the removal of the view makes that functionality impossible, you just write it differently, and move some logic from the accessor getters elsewhere. In my view, the use of accessors to make hidden AJAX calls is too tricky for my taste, and adds little to the model's functionality. I have no problem with storing promises in the accessors, or providing latches to load lazy associations, but try to avoid too much magic, particularly in accessors. But even if you go that route, no problem. Since any access to an associated property ultimately returns the promise, all you need to do is "handle" the promise.

So my first point is that lazy loading, however implemented has nothing to do with promises. While you clearly get it, others may be confused by the use of the term "deferred" is used, promises do not defer work its that their resolution may or may not come later in time. The defer bullies on and processes asynchronously, but isn't lazily waiting -- when its done, the value is resolved. But the promise may be resolved before ANY handlers are registered, or after all the handlers are registered, or a little bit of each. I wouldn't conflate the issues in this example.

The question is how to load that information into a view. Without auto-unwrapping of promises, the general model is that an object to be displayed works substantially as follows:

displayThing = 'default';
p.then(function(val){displayThing = val;})

When you have nested thins that depend on val, you can just

displayThing = 'default';
subDisplayThings = 'defaults';

getTopLevel().then(function(val){
  if (valArrivedOKButSucks(val)) throw 'val arrivedOK but sucks';
  displayThing = val;
  return getSubThings(val);
}).then(function(subvals){
  if (subvalsArrivedOKButSucks(val)) throw 'subvals arrivedOK but suck';
  subDisplayThings = subvals;
}).then(undefined, function(error){
  handleErrors(error);
})

Of course, that code ignores the fact that some objects are lazily loaded. In practice, this is not a problem, although the association's lazy functionality is triggered in your code by ANY REFERENCE to the associated property. This necessitates some need to put an "if" statement where appropriate to avoid the references, but that is a design decision that can be avoided in various ways that should be apparent here. No problem, however you choose to do it.

As an aside, I note that this process can be automated and reduced to simple calls by adopting conventions in the use of property and display names. Of course, using the controller itself in an angularJS expression is a simple call, but it doesn't always do what you want it to do, and you end up writing code to handle the default and watchers for databinding cases anyway.

I understand and appreciate the "syntactic sugar" sweetness from using the getter for lazy access and the promise as the value. But neither are necessary, nor necessarily clearer ways to write the code. As with, for example, Rails ActiveRecord objects, an awful lot happens under the covers to make that happen, and when things go wrong, its not always obvious what went wrong. What is more, its a royal pain to test when the various pieces of this magic are actually behaving as you expect. I don't disagree, and actually implemented something very much like that for a model in one of my projects.

The difference is that the logic for displaying the code must now be specified expressly, probably in the controller but possibly as a function in the service. The presence or absence of "nested promises" as you call them (and clarify by the use in the plunker) has nothing to do with that. You can code it, you just code it differently. When the default behavior acts as you describe, you would not need to write such code -- but the practical solutions often require more, leading to a creeping featuritis arising from what is actually an abuse of the promise.

@lyschoening
Copy link
Author

@wizardwerdna Thanks for your detailed response.

Just to confirm, with nested promises I meant promises returning regular objects with one or more promises in the attributes. I appreciate your explanation of the design considerations behind removing this option. However, would say that promise unwrapping is more than just sugar.

There are some downsides to having to explicitly resolve promises when you have data in sub-collections or when objects in a collection that have cross-references to other objects that cannot all be loaded in bulk. I do not think those are very exotic data structures.

Your example with chaining promises that works well, but once you have a promise that returns a collection of items, this approach fails.

This sort of works (single book, as per your example):

load(bookUrl).then(function (book) {
  $scope.book = book;
  return load(book.authorUrl);
}).then(function (author) {
  $scope.book.author = author
})

This on the other hand is painful (book collection):

load(booksUrl).then(function (books) {
  $scope.books = books;
  angular.forEach(books, function (book) {
      load(book.authorUrl).then(function (author) {
         book.author = author;
      });
   });
})

But there would still be the problem that promises will load each time the view is loaded; perhaps they would even load multiple times in parallel if the view changes rapidly. So the chaining becomes even more complicated:

load(booksUrl).then((books) {
  $scope.books = books;
  angular.forEach(books, function (book) {
      if(!book.author || book._authorLock) {
         book._authorLock = true;
         load(book.authorUrl).then(function (author) {
            book.author = author;
            delete book._authorLock;
         });
   });
})

The only sane solution in Angular 1.2.x is pairing each ng-repeat with an ng-controller that does the loading.

In my previous example that would look roughly like this (http://plnkr.co/xpWHfNSqYpkBUxXAI3Vn). There also is the option of not trying to remember the results of the nested promises at all (http://plnkr.co/edit/7V5DzvBXJsGGo1pPyrhE). That solution is of course reasonably clean, but it of course still requires additional controllers and it does not store results.
Let's say we don't want to write all the storage logic in every controller and we also don't want to 'forget' the results when the controller is unloaded. In that case, the most practical solution is to keep everything exactly the same as in my original example and to copy the results of the promises into the scope (http://plnkr.co/edit/cU34OsbxIbpMUW5ATGEB). This, however, means that there is absolutely no logic left in the additional "ngRepeat controllers". And that goes back to my original comment. My application frequently uses ng-repeat on sub-collections that are loaded by promise, some of it is graph data which has to be lazy loaded (via sugar or not) and some of it are just cross references. Promise auto-unwrapping is an elegant way of handling promises on object attributes. I still think it should remain an option.

There may be situation where promise auto-unwrapping is not desirable, but there are also situations where it is desirable. And there are many ways of making promise unwrapping opt-in without removing the feature altogether.

@wizardwerdna
Copy link
Contributor

There is a far more sane solution: promise aggregation. While bigger promise libraries have a much larger menagerie of these, $q has the one you need most: $q.all.

$q.all takes an array, object, promise for an array, and promise for an object and returns a promise that resolved if and only if all the contained promises (using your terminology) are themselves resolved. If all the promises are resolved, then the aggregate promise's handlers will be called with a value, as follows:

if the aggregate is an array, the value passed to the handler is a clone of the array, with promises replaced by their values. If the aggregate is an object, the value passed to the handler is a clone of the object, with promises replaced by their values. Thus:

d = $q.defer

arr = $q.all([1,2,d.promise]);
obj = $q.all({one: 1, two: 2, three: d.promise})
arr.then(logger)
obj.then(logger)

d.resolve(3);

should log [1,2,3] and {one:1, two: 2, three: 3}, respectively. This function only operates for unwinding shallow promises, which is to say that if the array has subarrays containing promises, You can cherry pick the aggregate function of your choice from other libraries or code your own to suit your needs. The unwinding here is shallow only, in that it operates on array elements that are promises, or object properties that are promises, but not more deeply nested promises -- that you would need to do yourself. (Note that your application already does that descent anyway in its "parsing" function -- you can simply return $.all promises on the parents as you descend.)

I'm sure it is apparent how this fairly directly solves your problem of relating nested objects with promises to the view. I hope it also demonstrates how specific aggregate implementations may be required for various solutions. $q.all is the only aggregate provided (check out the documentation for $q, when, covenant and the like to see how many different solutions exist for various applications, as well as async for analogous modes of aggregation).

Some people wondered why the $.all function was provided, since there is an obvious trivial implementation. Its because they didn't do the obvious trivial implementation, but packed a lot of power under the hood. There is a lot of hidden power in when as well. $.all is a beauteous and powerful thing, would you agree?

@wizardwerdna
Copy link
Contributor

In your specific example, $.all yields a lot of power, and promises provide an excellent way to unhide the logic of your code. For example, you might choose not to bury the "parse" code in the getBook function, but keep it separate. Thus, your code could be written:

getBooksAsJson($routeParams.foo)
   .then(parseJsonEncodedAssociations)
   .then(bindtoView)
   .catch(handleErrors)

where parse does your instrumenting Json as promises with additional .alls as described, bindToView does a simple assignment of the result to scope, and handleErrors handles errors at every level, including errors arising during parse (simply handled with a throw!).

Of course, if you want to "hide" the instrumenting in a getBooks functions, its even easier, although you will need to handle errors in the getBooks as a rejection of the promise, rather than a simple throw, or do the nesting in getBooks:

getBooks($routeParams.foo)
  .then(bindtoView)
  .catch(handleErrrors)

What's so insane about that?

@lyschoening
Copy link
Author

I did not know that $q.all() also works with regular objects (I've only used it with arrays of promises). That is certainly useful. I have of course looked at $q.all() before. It would potentially be my solution for those non-lazy promises (i.e. those that fire immediately when an object is loaded). That said, this solution would be what I referred to as 'blocking' in my original comment: It is not as good as what you can do currently. In the context of the example: I don't mind rendering the book before the author, however I would mind having to wait for the page to render while relatively minor annotations load. In my real world application there are quite a few such annotations and it would take too long to wait until they are all loaded.

There is an approach that ngResource uses, which works reasonably well with the 1.2.x templates (Plunker).
NgResource returns empty objects ([],{}), stores the promise in obj.$promise, and fills the objects with content whenever the promise resolves. This works well in the templates even in 1.2.x (not sure how the scope manages to auto-watch attributes that have not yet been set, but it does work). The ngResource approach is very fickle in the controllers because you never know if you have (1) a regular promise, (2) an object that has a promise on a reserved attribute, or (3) just a plain object. But perhaps that is the cleanest workaround; the confusion can be resolved with a custom $when implementation.

@wizardwerdna
Copy link
Contributor

Here's the thing about aggregate functions like $.all. None of these are
"special," they just use promises in the ordinary way -- you can always
write your own. I would suggest looking through the documentation in $Q
for inspiration, and simply consider writing your own aggregate operations,
tuned to your application.

There are no limitations that cannot be overcome, and you never suffer the
lack of a sane solution. If you need to manage lazy loading, write one that
respects the lazy promises and does not load them. Remember, you are
pulling in your JSON and "parsing" it -- your code can do whatever you
want. Don't blind yourself to one solution or one set of "tricks", such as
the getter/setter solutions. I am sure you can see how this can be done,
if you just put your mind to it.

On Tue, Dec 3, 2013 at 1:45 AM, Lars Schöning [email protected]:

I did not know that $q.all() also works with regular objects (I've only
used it with arrays of promises). That is certainly useful. I have of
course looked at $q.all() before. It would potentially be my solution for
those non-lazy promises (i.e. those that fire immediately when an object is
loaded). That said, this solution would be what I referred to as 'blocking'
in my original comment; it is not as good as what you can do currently. In
the context of the example: I don't mind rendering the book before the
author, however I would mind having to wait for the page to render while
relatively minor annotations load. In my real world application there are
quite a few such annotations and it would take too long to wait until they
are all loaded.

There is an approach that ngResource uses, which works reasonably well
with the 1.2.x templates (Plunker) http://plnkr.co/hr7UmaAOUtdBoo2ddQ5F.
NgResource returns empty objects ([],{}), stores the promise in
obj.$promise, and fills the objects with content whenever the promise
resolves. This works well in the templates even in 1.2.x (not sure how the
scope manages to auto-watch attributes that have not yet been set, but it
does work). The ngResource approach is very fickle in the controllers
because you never know if you have (1) a regular promise, (2) an object
that has a promise on a reserved attribute, or (3) just a plain object. But
perhaps that is the cleanest workaround; the confusion can be resolved with
a custom $when implementation.


Reply to this email directly or view it on GitHubhttps://github.com//issues/5153#issuecomment-29695691
.

@btford
Copy link
Contributor

btford commented Dec 19, 2013

I'm going to close this since this seems to be resolved.

We've thought about automatic promise unwrapping quite a bit and decided that it introduces too many complexities in the general use case to be worth it. One of the cool things about Angular's DI is that you can replace internal services if you so desire. So even though we've decided against this feature, you're welcome to maintain your own implementation of $parse that addresses your specific needs, @lyschoening.

@btford btford closed this as completed Dec 19, 2013
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants