angular-facebook
angular-facebook copied to clipboard
Possible reorganization of proxying and promises
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.
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
.
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.
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.
But in addition, something like #64 wouldn't possible when specifying the parameters the hard way.
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.