angular-facebook icon indicating copy to clipboard operation
angular-facebook copied to clipboard

Possible reorganization of proxying and promises

Open gafilson opened this issue 10 years ago • 5 comments

Hey there,

I've recently rewritten parts of the angular-facebook to use promises in what feels like a more standard, angularjs way. For example instead of:

Facebook.getLoginStatus(function (data) {
  if (data.status === 'unknown') ...
})

You would use:

Facebook.getLoginStatus().then(function (data) {
  if (data.status === 'unknown') ...
})

This probably seems subtle on the surface but in practice it makes a big difference as you can then can actually chain facebook promises together with other promises. It also keeps the execution of code upon response of facebook APIs within the angular context so no need for $timeouts or $scope.$apply etc.

When working with async resources like Facebook in complex ways it's really critical to be able to chain promises with other promises.

Let me know if you would like me to generate a pull request for my changes.

gafilson avatar Jun 14 '14 21:06 gafilson

As you can see here the function is already working with promises. So it should probably work to use the syntax like Facebook.getLoginStatus().then.

mrzmyr avatar Jul 31 '14 19:07 mrzmyr

Promises only work as long as a dummy callback is given. So it would be Facebook.getLoginStatus(function(){}).then(…); For login it’s Facebook.login(function(){}, {scope: 'email'}).then(…) , while a promise based approach would look like Facebook.login({scope: 'email'}).then(…)

The search for userFn would need to insert a promise callback if none given for this to work and maybe expect the userFn at a specific place instead of searching the whole arguments array. If it isn’t first or last, depending on the SDK method, it doesn’t work anyway.

I’ll check if I find some free time to provide a PR, but if someone else is quicker I wouldn’t mind.

nedt avatar Aug 26 '14 08:08 nedt

I currently work on some tests for the service and through about it, too. I will look it up after finishing the initial test suite for the service. PR is welcome for sure, tho.

mrzmyr avatar Aug 26 '14 08:08 mrzmyr

But in addition, something like #64 wouldn't possible when specifying the parameters the hard way.

mrzmyr avatar Aug 26 '14 15:08 mrzmyr

I'd also be happy to see a more promise-friendly syntax.

Seems you wouldn't be able to loop through function names as you do now, and would have to write more specific wrappers for the FB.* functions, taking into consideration whether they take functions as arguments and in what position.

maackle avatar Jul 24 '15 06:07 maackle