Do not force to use require() to create a new fbgraph instance

[louisameline]

Hello and thank you for the good work on this module.

A suggestion : it would be nice if we could require the module once at the beginning of our scripts instead of everytime we to open a connection to Facebook.

Unless I missed something, right now we have to write :

// somewhere deep down in my script
$.each(contacts, function(i, contact)){
    // create a dedicated instance of fbgraph (we don't want only one instance which would get a different access token every now and then)
    var graph = require('fbgraph');
    graph.setAccessToken(token);
    graph.someMethodWithAuth({
        "client_id": conf.client_id,
        "client_secret": conf.client_secret
    };
    graph.doSomething()...
}

I'd rather do something like

// at the top of my file with all the others, for clarity.
// double underscores are my own convention to show that the variable is a module in the "global" scope
var __fbgraph = require('fbgraph');
// global default config
__fbgraph.config({
        "client_id": conf.client_id,
        "client_secret": conf.client_secret
    };

// later, deep down in my script
$.each(contacts, function(i, contact)){
    var graph = new __fbgraph();
    graph.doSomething()...
}

Also, I see no support of the batch get requests, which is going to be a deal breaker for me. Thanks anyway :)

[criso]

there's no reason for you to have to do:

$.each(contacts, function(i, contact)){
    var graph = new __fbgraph();

In your example, you already have access to __fbgraph. You should never have to do:

inside a loop.

Batch requests are get requests with more params. https://developers.facebook.com/docs/graph-api/making-multiple-requests/

Anything you can do with curl you can do with fbgraph.

[louisameline]

Sure but what I find unconvenient is that there is only one global accessToken variable in the module, or am I wrong ? Forgive me if I am, I have little time these days to investigate much. Ok, take this function :

function contact_post(contact) {
    graph.setAccessToken(contact.accessToken);
    graph.post("/feed", { message: "Message 1" }, function(err, res) {
        graph.post("/feed", { message: "Message 2" }, function(err, res) {

        });
    });
}

Imagine that this function is called for contact1. While Facebook is taking some time to answer, imagine the function is called for contact2 : the access token variable is going to be overwritten with contact2's access token. When Facebook answers, the second call to graph.post() for contact1 is made, but it will fail because of the wrong access token. So that forces us to set the access token before every call, which is annoying.

That's why I prefer to create one fbgraph object per contact, using the require inside a loop (I know, I don't like that either, I'd rather have a proper method). That way, each object represents a separate connection to Facebook and will not interfere with the others. And incidently, the credentials can be set once only. It seems more intuitive and all to me.

As for batch requests, I can't see how you handle them in fbgraph, can you give me an example ? If you mean that I can build the whole request myself, well sure, but isn't that the point of a library ? What I expect is this (quoted from a library similar to yours) :

FB.api('', 'post', { 
    batch: [
        { method: 'get', relative_url: '4' },
        { method: 'get', relative_url: 'me/friends?limit=50' }
    ]
});

[louisameline]

Ok, forget the batch request thing sorry, I see your lib can do the same. Maybe just an example in the doc would be welcome, since it's the first thing I looked for in the readme and couldn't find an answer straight away. Thank you.

[criso]

Cool, so:

1. https://github.com/criso/fbgraph#to-use-a-specific-access-token-for-a-particular-request
2. I didn't add an example on purpose. This lib is a proxy to the facebook api so that it's flexible. If facebook changes their batch api tomorrow, this lib isn't affected by it. In order to do what you want with the api you still have to look at the facebook documentation.

Basically following the same idea as: https://developers.facebook.com/docs/javascript/reference/FB.api Libs that create specific methods are bound to fail if they're not consistently in sync with their endpoints. Trust me, you don't want to wait for a new fbgraph release just because facebook changed something.

[louisameline]

1. Well, the objective for me is precisely not to have to specify the access token each time. I feel creating an instance for a contact which would be used for a series of request on him would be elegant, but that's just me and that was just a suggestion.
2. Ok, it makes some sense, although it's surprising coming about a library that tampers the results of some API calls :) Still, I think people who use libraries (at least people like me) precisely want to avoid the hassle of having to read documentation of the resource to use it, they want minimum work and useful info right upfront. And Facebook's doc is not awful, but it's certainly not the best either.

Last -respectful- rant for today, about your lib tampering the /user_id/picture response in :

graph.get("/zuck/picture", function(err, res) {
  console.log(res); // { image: true, location: "http://profile.ak.fb..." }
});

You are probably aware that people who do not want the facebook redirection may specify a redirect parameter set to 0 ? See https://developers.facebook.com/docs/graph-api/reference/user/picture/ So I think the lib should send the picture resource for this one, not home-made json. It's ok for an API to return anything other than JSON really.

Thank you

[criso]

1. You don't have to specify it each time. The access token is static. You specify it once or per request. Oh, yeah, I see what you're saying. You want a series of fbgraph objects. Yeah, this is setup to be static to make requests. I believe someone at some point forked this lib with that intention.
2. I see your point. Wanted to keep the responses consistent. Didn't want to add clauses like "if you request a picture it'll do this.. if you request video, it'll do something else"

[louisameline]

I'm going to be a pain in the ass, but for the sake of the debate I'll explain why I'm not a fan of your approach :

By tampering the results, I know you want to create consistency so we do not have to care about the format of what Facebook actually sends, and save us the hassle of writing specific code for it. But the result is, we can't fully trust Facebook's documentation anymore, according to which request we are making. Instead, we have to learn the schema of the json that your library sends back in these exceptional cases. In the end, instead of specific code for Facebook, we have to write specific code to adapt to your library's response. In this case, I feel like I lose more than I win, but that's just me. Thanks anyway for your work, it's appreciated.

[criso]

Valid point but this was written a while a ago and while facebook's docs may have gotten better, I've learned not to trust them regarding backwards compatibility.

Also, you're not being a pain in the ass. You have strong opinions about how this should work. Sounds like a fork is in order.

[louisameline]

I'm not sure about a fork, but thank you for your answers. Have a nice day Cris.

[criso]

You too dude