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

feat($http): allow data promises in $http requests #1676

Closed

Conversation

gonzaloruizdevilla
Copy link
Contributor

It's possible to bind directly the promise returned by $http to the $scope model so I think it would look natural to do contrary, to pass to $http a data value that is a promise.

I mean something like this:

function MyCtrl($scope,$http){
    $scope.something = $http.get("geturl");
    $scope.update = function(){
        $http.post("posturl", $scope.something)
            .success(doSomething)
    }
}

In this pull request I have created a wrapper around $http to handle the situation in which data is a promise.
I have added two unit tests to check this feature.

Signed-off-by: Gonzalo Ruiz de Villa <gonzaloruizdevilla@gmail.com>
@petebacondarwin
Copy link
Contributor

I am nervous about automatically processing promises. There is hidden magic that makes code less readable, especially for people reading the code for the first time. There has been a lot of confusion around this functionality with the scope.

Currently, this would be done as follows:

function MyCtrl($scope,$http){
  $scope.something = $http.get("geturl");
  $scope.update = function(){
    $scope.something.then(function(request) {
      $http.post("posturl", request.data).success(doSomething);
    });
  }
}

which gives full control of the data that is passed and is obvious to the reader what is happening. Also, allows you to deal with

@gonzaloruizdevilla
Copy link
Contributor Author

I understand your points, and I agree most with the first one about automatically processing promises.

However, what confused me was the opposite. I started loading the scope with data that were promises. When I made the code that posted the data to the server I found that the request payload was empty. Of course, I found that this happened because I was passing to the $http service a promise and not the real data. I found it a little bit disturbing, as it seemed to me that it broke symmetry: I can't send back to the $http service what it did give me in first place. In my opinion, this looks like an unexpected behavior, but the behavior should be what the majority expects, and not only me.

@gonzaloruizdevilla
Copy link
Contributor Author

In the event that this looked interesting, I would modify de PR, because I forgot to include the code to manage the case when the promise of data is rejected. I would also make a small refactor to remove duplicated code.

@pkozlowski-opensource
Copy link
Member

@gonzaloruizdevilla thnx for your work but I'm afraid that I'm with @petebacondarwin here. I find this approach very confusing. $http makes it very clear that the promise is returned from its methods invocation. Actually the fact that templates are showing values from the resolved promises is nice but it is "magical" as well. On top of this the automatic resolution won't work if a promise is returned from a function, see: #990

Chaining promises with then is very readable, explicit and doesn't add bits to the framework.

In short: 👎 if you ask me

@gonzaloruizdevilla
Copy link
Contributor Author

Ok, thank you for your comments. Also, I'm must acknowledge that storing promises in the $scope looks simple, but when accessing the real values from other functions inside the controller to modify the model, you have to fill this functions with "then"s that make a lot of noise and means more code.

Anyway, this was a nice exercise to learn how to make PR to the angularjs project.

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.

3 participants