New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extra digest required if Angular >= 1.1.4 #71

Closed
mokesmokes opened this Issue May 26, 2013 · 15 comments

Comments

Projects
None yet
6 participants
@mokesmokes

mokesmokes commented May 26, 2013

Same bug (unless it's an undocumented "feature") as $http/$resource.
Lots of reports on Angular issue list due to this.
http://plnkr.co/edit/DZmWNi?p=preview
See that the XHR is not sent if Angular version >= 1.1.4 unless a digest cycle performed.
Change Angular and angular-resource to 1.0.7 and all is good.

@mgonto

This comment has been minimized.

Show comment
Hide comment
@mgonto

mgonto May 26, 2013

Owner

Hey,

I agree that this sucks, but Angular's best practices do recommend that when you're getting a callback from an "unknown" event to Angular, you should actually call $scope.$apply to perform a new digest cycle. If you do that, you wouldn't have this error, and if you're using Angular 1.0.7 you wouldn't have this error either when doing the $apply as it checks wether there's already a $$phase running.

So I think there's an easy solution for this. I do agree that this is not a "feature", but it does make sense to call the $apply after any callback that angular doesn't know about, don't you agree?

If they do change this in Angular >= 1.1.4, it'll work here as well. I could check for Angular's version with some hacky thing and call the $apply to perform an extra digest before your callback but I don't know if this makes sense to do it. What do you think?

Thanks for the report!

Owner

mgonto commented May 26, 2013

Hey,

I agree that this sucks, but Angular's best practices do recommend that when you're getting a callback from an "unknown" event to Angular, you should actually call $scope.$apply to perform a new digest cycle. If you do that, you wouldn't have this error, and if you're using Angular 1.0.7 you wouldn't have this error either when doing the $apply as it checks wether there's already a $$phase running.

So I think there's an easy solution for this. I do agree that this is not a "feature", but it does make sense to call the $apply after any callback that angular doesn't know about, don't you agree?

If they do change this in Angular >= 1.1.4, it'll work here as well. I could check for Angular's version with some hacky thing and call the $apply to perform an extra digest before your callback but I don't know if this makes sense to do it. What do you think?

Thanks for the report!

@ghost ghost assigned mgonto May 26, 2013

@mokesmokes

This comment has been minimized.

Show comment
Hide comment
@mokesmokes

mokesmokes May 26, 2013

Well, to be honest I don't share this view. What I have believed was that you should $apply when you want to update bindings outside the Angular context. BUT in the case of $http and $resource you care about the bindings only when the data comes back from the server! (And in that case, both of them don't fail of course, regardless of when they were called). I find it incredibly weird that we need to call $apply just to get the request out to the server.

So the implications for Restangular? I don't have an easy answer, actually. I definitely don't think that the developer should call $apply to send a request to the server - there is no sense in that, and if $resource persists with this "feature"/"bug" then it would be great if you can do away with this for the rest of us, somehow. In either case, for all of $http, $resource, and Restangular none of this is documented behavior - so in my eyes it's a bug either way - of documentation or design/code.

So even if you just wanted to document it - I'm not sure what you should write, to be honest.

mokesmokes commented May 26, 2013

Well, to be honest I don't share this view. What I have believed was that you should $apply when you want to update bindings outside the Angular context. BUT in the case of $http and $resource you care about the bindings only when the data comes back from the server! (And in that case, both of them don't fail of course, regardless of when they were called). I find it incredibly weird that we need to call $apply just to get the request out to the server.

So the implications for Restangular? I don't have an easy answer, actually. I definitely don't think that the developer should call $apply to send a request to the server - there is no sense in that, and if $resource persists with this "feature"/"bug" then it would be great if you can do away with this for the rest of us, somehow. In either case, for all of $http, $resource, and Restangular none of this is documented behavior - so in my eyes it's a bug either way - of documentation or design/code.

So even if you just wanted to document it - I'm not sure what you should write, to be honest.

@mgonto

This comment has been minimized.

Show comment
Hide comment
@mgonto

mgonto May 26, 2013

Owner

Hey,

Thanks for the input.

I agree that this is kind of weird. I'll try to make a fix that doesn't break 1.0.7 and that's not to hacky.

Owner

mgonto commented May 26, 2013

Hey,

Thanks for the input.

I agree that this is kind of weird. I'll try to make a fix that doesn't break 1.0.7 and that's not to hacky.

mgonto added a commit that referenced this issue May 28, 2013

Added Restangular version support for AngularJS
Fixes #69

Adds comment regarding #71. I'll see how to fix this.
@mgonto

This comment has been minimized.

Show comment
Hide comment
@mgonto

mgonto Jun 6, 2013

Owner

For now added to the documentation.

I've not found a clean way of fixing this unfourtenately, which sucks.

Please reopen if you have any recommendation on how to fix this without changing Angular's code.

Owner

mgonto commented Jun 6, 2013

For now added to the documentation.

I've not found a clean way of fixing this unfourtenately, which sucks.

Please reopen if you have any recommendation on how to fix this without changing Angular's code.

@mgonto mgonto closed this Jun 6, 2013

@sujoy18

This comment has been minimized.

Show comment
Hide comment
@sujoy18

sujoy18 Nov 1, 2013

'_' is undefined from restangular I am getting this error while using it in my application. Any help?

sujoy18 commented Nov 1, 2013

'_' is undefined from restangular I am getting this error while using it in my application. Any help?

@mgonto

This comment has been minimized.

Show comment
Hide comment
@mgonto

mgonto Nov 1, 2013

Owner

Include lodash or underscore before


Mobile mail. Excuse brevity and typos.

On Fri, Nov 1, 2013 at 12:58 AM, sujoy18 notifications@github.com wrote:

'_' is undefined from restangular I am getting this error while using it in my application. Any help?

Reply to this email directly or view it on GitHub:
#71 (comment)

Owner

mgonto commented Nov 1, 2013

Include lodash or underscore before


Mobile mail. Excuse brevity and typos.

On Fri, Nov 1, 2013 at 12:58 AM, sujoy18 notifications@github.com wrote:

'_' is undefined from restangular I am getting this error while using it in my application. Any help?

Reply to this email directly or view it on GitHub:
#71 (comment)

@sujoy18

This comment has been minimized.

Show comment
Hide comment
@sujoy18

sujoy18 Nov 1, 2013

Thanks for your reply. I am very new in angular js. Could you please explain exactly where to add. I am getting this error while loading the application.

sujoy18 commented Nov 1, 2013

Thanks for your reply. I am very new in angular js. Could you please explain exactly where to add. I am getting this error while loading the application.

@julien-rodrigues

This comment has been minimized.

Show comment
Hide comment
@julien-rodrigues

julien-rodrigues Nov 1, 2013

Include it like any other javascript files using script tag but before calling the restangular service file

julien-rodrigues commented Nov 1, 2013

Include it like any other javascript files using script tag but before calling the restangular service file

@mruzekw

This comment has been minimized.

Show comment
Hide comment
@mruzekw

mruzekw Nov 17, 2013

Is this still an issue with AngularJS 1.2.1?

mruzekw commented Nov 17, 2013

Is this still an issue with AngularJS 1.2.1?

@sujoy18

This comment has been minimized.

Show comment
Hide comment
@sujoy18

sujoy18 Nov 18, 2013

I didn't use it in project so cant tell whether it is working with 1.2.1 or not.

On Monday, November 18, 2013 5:21 AM, Will Mruzek notifications@github.com wrote:

Is this still an issue with AngularJS 1.2.1?

Reply to this email directly or view it on GitHub.

sujoy18 commented Nov 18, 2013

I didn't use it in project so cant tell whether it is working with 1.2.1 or not.

On Monday, November 18, 2013 5:21 AM, Will Mruzek notifications@github.com wrote:

Is this still an issue with AngularJS 1.2.1?

Reply to this email directly or view it on GitHub.

@mgonto

This comment has been minimized.

Show comment
Hide comment
@mgonto

mgonto Nov 18, 2013

Owner

@mruzekw I don't know either unfortunately, but if it's it's an issue for $http, $resource an Restangular.

Owner

mgonto commented Nov 18, 2013

@mruzekw I don't know either unfortunately, but if it's it's an issue for $http, $resource an Restangular.

@Sarabadu

This comment has been minimized.

Show comment
Hide comment
@Sarabadu

Sarabadu Nov 20, 2013

i solved whit this for now:

i change this:

var foo= Restangular.all('foo');
$scope.foo= foo.getList();

to this:

var foo= Restangular.all('foo');
foo.getList().then(function(result){
   $scope.foo= result;
});

but this keep your $scope.foo not restangularizer so...

Sarabadu commented Nov 20, 2013

i solved whit this for now:

i change this:

var foo= Restangular.all('foo');
$scope.foo= foo.getList();

to this:

var foo= Restangular.all('foo');
foo.getList().then(function(result){
   $scope.foo= result;
});

but this keep your $scope.foo not restangularizer so...

@mgonto

This comment has been minimized.

Show comment
Hide comment
@mgonto

mgonto Nov 20, 2013

Owner

You need to do that in Angular > 1.2 because promise unwrapping was depracated.

But result should be restangularized

Owner

mgonto commented Nov 20, 2013

You need to do that in Angular > 1.2 because promise unwrapping was depracated.

But result should be restangularized

@Sarabadu

This comment has been minimized.

Show comment
Hide comment
@Sarabadu

Sarabadu Nov 20, 2013

you are right, i correct my comment for avoid confusions

Sarabadu commented Nov 20, 2013

you are right, i correct my comment for avoid confusions

@mgonto

This comment has been minimized.

Show comment
Hide comment
@mgonto

mgonto Dec 9, 2013

Owner

I've implemented something to fix this promise unwrapping problem.

Check out https://github.com/mgonto/restangular#using-values-directly-in-templates

Owner

mgonto commented Dec 9, 2013

I've implemented something to fix this promise unwrapping problem.

Check out https://github.com/mgonto/restangular#using-values-directly-in-templates

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