Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to use RegExp object to define routes #52

Open
john-doherty opened this issue Jan 15, 2017 · 2 comments
Open

Unable to use RegExp object to define routes #52

john-doherty opened this issue Jan 15, 2017 · 2 comments

Comments

@john-doherty
Copy link

I'm trying to use:

router.ws(new RegExp('^/([0-9]{6})$'), function (ws, req) {
// do stuff
});

But it's throwing an error on line 18 of websocket-url.js because it's always assuming a string:

/* The following fixes HenningM/express-ws#17, correctly. */
function websocketUrl(url) {
  if (url.indexOf('?') !== -1) {
    var _url$split = url.split('?');

    var _url$split2 = _slicedToArray(_url$split, 2);

    var baseUrl = _url$split2[0];
    var query = _url$split2[1];


    return (0, _trailingSlash2.default)(baseUrl) + '.websocket?' + query;
  }
  return (0, _trailingSlash2.default)(url) + '.websocket';
}
@joepie91
Copy link
Collaborator

Hmm, this is actually a non-trivial thing to fix.

The problem is that to integrate with Express, express-ws will create a 'fake' suffixed GET route that applies all the specified middleware, before actually doing anything with the WebSocket. To create this fake route, it must add a suffix like .websocket at the end, to make sure it doesn't conflict with existing routes.

However, in the case of a regular expression, you can't just "add a string at the end". While it might be possible to implement this, it would involve a very carefully written regex-aware implementation of the suffixing code.

This is complicated by the fact that Express has multiple route formats, not all of which are documented, including a string-based regex-containing one.

I might take a crack at this in the future when I can find the time, but for now the easiest workaround would be to just use string-based route parameters, and handle 'validation' of the parameters within the route itself.

@john-doherty
Copy link
Author

john-doherty commented Feb 23, 2017

I agree @joepie91 and that's what I've done for now. I did consider updating the code and issuing a pull request, but like you, I realized it was going to be quite a bit of work and require a good few unit tests.

Likewise, if I find some free time I'll take a look at it - I'll let you know first of course.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants