jesseditson / fs-router

Use the FS as your micro router
BSD 3-Clause "New" or "Revised" License
165 stars 19 forks source link

Error on Windows #4

Closed RemyLespagnol closed 7 years ago

RemyLespagnol commented 7 years ago

Hi @jesseditson,

They are some issues on Windows.

First, create folder like :params it's not permitted by default on windows. And the second, it's that on Windows your router doesn't work !

It's because of Windows' paths. It's like \\foo\\bar So when you have r.match(req.url), the regex doesn't work.

on line 58 you can replace this : route.path = '/' + path.relative(routesDir, routeFile).replace(extPattern, '')

by this :

route.path = '/' + path.relative(routesDir, routeFile).replace(extPattern, '')
// Fix for windows users
route.path = route.path.replace(/\\/, '/');

So the regex is right and your router works :)

PS: I can't make a make pull request because of the folder's name in your test folder.

RemyLespagnol commented 7 years ago

Oh sorry.... I don't look at the PR.

jesseditson commented 7 years ago

No worries! (PR is #2 for posterity) - I'm waiting on an update to merge that PR, but could use your opinion as a windows user - what's a reasonable default other than : to indicate that the path is a parameter? I'd like to keep this library fairly opinionated and avoid too much configurability. If you and @jpbourgeon can agree on a prefix, I'm sure he'd be happy to update that PR and we can land windows support.

RemyLespagnol commented 7 years ago

Fo the param pattern, on Windows I don't see any best practices for this kind of things.

On Windows, to echo an environment variable, we use %VAR%. Named a folder with % is authorized.

So maybe % or _ can be good.

jpbourgeon commented 7 years ago

Hi

@Themandunord : your opinion would be welcome on the PR too #2

jpbourgeon commented 7 years ago

I agree that % might be a good option on windows if I had to make an opinionated choice.

jesseditson commented 7 years ago

Resolved via #2 released in v0.4.0