search

visionmedia / express-resource

Resourceful routing for Express
1.41k stars 140 forks source link

load does not have access to request for other auto-loaded variables #31

Closed jupiter closed 13 years ago

jupiter commented 13 years ago

In the example /forums/5/threads/12 the 'forum' can be loaded in the auto-loading load method. In the load method for a 'thread' there's no way to access the already loaded 'forum' - which could contain either neccesary information to load the 'thread', or could in fact contain the 'thread'.

At the moment, the load method should look like function (id, callback) {} I don't see the need for passing the id value, rather than the request. It only hides the key to the param containing the id. I realise that changing the arguments for this would break existing implementations and add a line of code to these implementations, e.g. var id = req.params.forum;, but I can't see another way.

tj commented 13 years ago

yeah we could pass the request and make the sig (req, id, callback) or something

jupiter commented 13 years ago

Yes, we could, but I wouldn't want to break existing implementations. The only way to not to do that would be to make the sig (id, callback, req). What do you think?

tj commented 13 years ago

yeah im fine with that, passing the id is slightly useful "sugar" for simple cases I suppose, so I guess we dont have to remove it or move it for now

jupiter commented 13 years ago

Actually, I just pulled and found that you've already implemented it in backward-compatible way. (I didn't even realise you could do fn.length to get the function's number of arguments.) Good job! Now it just needs to go into the documentation and into a release. Hope there's one soon. Thanks.

tj commented 13 years ago

haha yeah just saw that too, forgot we had it