search

criso / fbgraph

NodeJs module to access the facebook graph api
http://criso.github.io/fbgraph/
1.09k stars 176 forks source link

graph api only accepts username but should accept userId also #4

Closed david-truong closed 12 years ago

david-truong commented 12 years ago

line 215 of your code

if (typeof url !== 'string') {
  return callback({ message: 'Graph api url must be a string' }, null);
}

not sure how you want to handle this.. whether to use an option to say its a userid and then ignore the check or just remove this check altogether.

david-truong commented 12 years ago

You may not want to fix it at all and just document it. I just passed my userId as a string after I looked at the code but I was confused at first why it wasn't working.

criso commented 12 years ago

Yeah, I don't really see that as error, since it's an url, which should* be a string.

The function has a comment stating what it expects, so I think adding separate documentation for it is over-kill

Could convert what is passed in to a string, but if and object is passed the only error triggered would be facebook's error.. and we all know how awesomely descriptive those are.