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

wildcard issue in group router #19

Open
noahehall opened this issue May 26, 2021 · 5 comments
Open

wildcard issue in group router #19

noahehall opened this issue May 26, 2021 · 5 comments

Comments

@noahehall
Copy link

noahehall commented May 26, 2021

unable to specify catchall route handler if a named route exist at the same level

issue:

  • it appears i cannot have a catchall route at the same level of a named route?

purpose:

  • I want to catch all invalid requests at URI good/good/badname

problem:

  • if I already have good/good/goodname
  • koa-tree router throws error `error in v1 handler Error:
  • **wildcard route 'notfound' conflicts with existing children in path '/v1/demo/notfound'`'
    //works
   v1RouterGroup.get('/', v1Handler);

   //works
    v1RouterGroup.get('/demo/pkgcheck', demo.pkgCheckHandler.getPkg);

   // does not work
    v1RouterGroup.get('/demo/*notfound', demo.pkgCheckHandler.notFound);

   //works
    v1RouterGroup.get('/demo/someothershit/*notfound', demo.pkgCheckHandler.notFound);

here is a link to the actual code

here is a link to the handler definition

@noahehall
Copy link
Author

link to line throwing code:

if (n.children.length > 0) {

@steambap
Copy link
Owner

I'll try to fix it during the weekend.

I think the original intension of the Golang implementation is that, there should be only one route for any path. With that guarantee, it is possible to use goroutine to do an optimization on the route search. Anyway, this is not relevant in Node.js and should be fixed.

@noahehall
Copy link
Author

noahehall commented May 27, 2021

@steambap dope for responding! i was just hacking my way through the source code ;)

if you have an idea of what your going to do fix it, send it my way and hopefully we can share the burden.

the information you've provided is helpful and clarifies why the error is there.

@steambap
Copy link
Owner

Hi @noahehall , if you know how Golang works, you can just copy everything, including the tests, from here:
julienschmidt/httprouter#329

@noahehall
Copy link
Author

noahehall commented May 27, 2021

I know golang about as good as I know swahili.

I reviewed julienschmidt/httprouter#329 and it doesnt quite solve this issue, however would still be a net benefit.

the issue fixed on httprouter is specific to routes with variable path segments, here we're concerned with catchalls

[update]
reviewing the patch that fixes the variable path segment issue provides a clear pathway for fixing this issue.

i’ll tackle it and submit a PR.

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

No branches or pull requests

2 participants