search

imrefazekas / connect-rest

Exceptionally featureful Restful web services middleware for connect node.js
MIT License
100 stars 29 forks source link

Setting context: '/' causes issues with URL #38

Closed reybango closed 8 years ago

reybango commented 8 years ago

Hi guys,

I'm working through a demo site and I was defining the API url as api.meadowlark:3000. So hitting an endpoint might look like http://api.meadowlark:3000/attractions

I defined the context as:

context: '/'

but doing so forced me to have to use an extra slash in the url like this:

http://api.meadowlark:3000//attractions

Defining the context as:

context: ''

fixed the issue so I didn't have to use an extra slash. I'm not sure if it's a bug or if it's expected behavior. The code can be referenced here:

https://github.com/reybango/meadowlark/blob/master/meadowlark.js#L178

imrefazekas commented 8 years ago

context: '' should work. it is a prefix added to the path in that version so path can not end with '/'

reybango commented 8 years ago

@imrefazekas thanks again for taking the time to look into this. This is a very popular intro book to Node.js & Express development and I'm sure you're happy to see your awesome work referenced. :)

I know you did a PR in @EthanRBrown's repo but you may want to explain why the context should be set to '' and not '/'. If you look at the code from his book, he does create the context with a slash

https://github.com/EthanRBrown/web-development-with-node-and-express/blob/master/ch15/meadowlark.js#L289

Not sure if that's an artifact of an older version of connect-rest when he originally wrote the book. I'll comment on that in the PR you made.

imrefazekas commented 8 years ago

Contact is a prefix like I wrote above. The final path will be a concatenation of context and path, so a context of '/' and a path of '/api' would result a '//api' string at the end leading to a wrong result. So if you have no context, set it as an empty string.

reybango commented 8 years ago

@imrefazekas yep totally get that. I guess what I'm questioning is why the book example would set the context to a "/" and not an empty string. Since the book referenced deprecated code (e.g.: rest.rester()), I was wondering if pre v2.0 code required the context to be "/".

imrefazekas commented 8 years ago

The book was using version 0.9.X something, that context thing changed in version 1.0 for sure. For '/' you should have 0.9.X I believe, however, that version was not 1.0, so much reduced as for features and won't have promises for sure ...

reybango commented 8 years ago

@imrefazekas got it. I had a feeling that was the case. Appreciate the feedback.