Allow creating instances

[brigand]

The idea is to maintain compatibility, but move all of the module level variables to instance properties.

This allows prototypical inheritance so that the setters don't affect other areas of the code. This basically shows the use case. Also... it's just bug prone to be calling these setX functions before every request.

// graph is a Client
var graph = require('fbgraph');
graph.setAppSecret('abc');

app.use(function(req, res, next){
  // create a Client that inherits from graph
  req.graph = graph.createClient();
  req.graph.setAccessToken(req.user.fbAccessToken);

  graph.getAccessToken() === null
  graph.getAppSecret() === req.graph.getAppSecret()

  graph.setAppSecret('def');

  // still equal because req.graph doesn't have an own appSecret
  graph.getAppSecret() === req.graph.getAppSecret()

  next();
});

[djelic]

Hi,

I agree that using setAccessToken for each request is not the best solution but I think that in your case, creating instance for each request is also not good. Certain calls to graph.facebook.com can take quite some time so if you have significant number of clients, instances can pile which could be memory intesive.
However, you can pass access token cleanly as param for each call like this:

graph.get('/me', { access_token: "..." }, cb)

Global access_token is only appended when not provided: link

[brigand]

That's fair. I'll create a thin wrapper around it that just ensures an access token is provided (for type checks).

It seems @criso isn't very active currently so I'll just leave this open for now.

Thanks for the tip.

[criso]

Sorry guys, it's been a crazy week and I wanted to let this marinate for a bit to see how everyone else feels about it.

Using objects for this seems like the same thing as creating a new request object each time you want to do ajax or hit a service.

If you're building requests for 10 different users, would you keep those in a pool ? How would you free up those objects later on? I'm wondering about things like that

[jan-osch]

It would be a very good feature. For example in one project I need to use multiple graph versions - now I am unable to do so.

[newhouse]

+1 for adding this.

If my app is doing stuff with the graph API for multiple users at once, it looks like the executions will happen on behalf of whatever User's token was passed to setAccessToken() last. This would be bad.
A simplified example:

const graph = require('fbgraph');

userOne.token = 'xxxx';
userTwo.token = 'yyyy';

graph.setAccessToken(userOne.token);
graph.get('/me', (err, res) => {
  console.log(res.id); // Probably User 1's id. Maybe not.

  // Exaggerating something *very* async here to illustrate, but could def happen without `setTimeout` depending
  // on how long this call took to resolve.
  setTimeout(() => {
    graph.get('/me', (err, res) => {
      console.log(res.id); // Almost certainly will be User 2 id due to delay.
      userOne.facebookId = res.id;
      userOne.save(); // userOne now has userTwo's facebook id?!
    });
  }, 5000);
});

// Here's where we start mixing things up
graph.setAccessToken(userTwo.token);
graph.get('/me', (err, res) => {
  console.log(res.id); // User 2 id
});

As mentioned by another comment, setting the token before each call is not really a great solution as it's prone to forgetfulness and is painfully verbose. As-is, this will cause a few things to happen:

1. Users see this problem upfront and choose another module over this one - that would be a shame because I like this!
2. Users don't realize what may happen under certain cases and they screw up their app data real bad and come in here flaming - also not good!

Thanks for the work on this so far!

[akhoury]

no progress here :(

[newhouse]

Not meant to be a snarky remark here, but I ended up switching to the fb library instead. Does what I need it to, and if this issue is a concern of yours, it may be a solution for you, as well @akhoury. Still very appreciative of the work on this library, so thank you very much!

[shashaBot]

For anyone who needs this, there is an elegant solution for this.

I like the simplicity of this library, so I created a es6 class around this library and passed the access_token in the constructor. Like this:

class FbGraph {
   constructor(token) {
      this.graph = require('fbgraph')
      this.graph.setAccessToken(token)
  }
}

Then, you can create instances of this class with different access tokens. I did this with es6 classes but the same can be achieved with prototype functions.