Add RegExp path support - #68
Conversation
…rve trailing slashes. Support optional express url params.
|
Thanks for your PR! I do have a few concerns, however:
|
|
I retained the existing .websocket suffix, it has not changed to a prefix - if you see otherwise, I'd appreciate if you could point this out in the code. The hash change was a simple enough modification of the regex - I have some apps that route URLs internally, and wanted to make sure that both ? and # would be properly supported. The trailing slash preservation is due to the express "strict" routing option, that differentiates URLs that end with slash from non-slash. It seemed prudent to add support for that particular case. |
|
Hi @deckerrj, First of all: this looks like a solid pull request. Neat & concise solution to what I assumed would require a lot more fiddling and ugly hacks. I haven't yet tested all the possible edge cases that have to be covered here, but so far it seems to work quite well. I am also a bit curious about point #2 from @joepie91's list though. Since this library is intended to be used on the server side I don't see any reason to include hash fragments as part of the URL RE. So if there's a legitimate reason for doing this then please let us know. Thanks for the contribution! |
|
@HenningM You bring up a fair point about the server side parsing. Since the hash element doesn't crop up in my internal routing I've added another commit to remove it. Let me know if you want me to squash the two together. |
Added support for modifying RegExp paths. Also fixed a few bugs, including: properly handling hash url elements, preserving trailing slashes in URLs, and supporting optional express url params (ala :param?)