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

Global route syntax causes parse error #246

Open
zone117x opened this issue Mar 16, 2021 · 5 comments
Open

Global route syntax causes parse error #246

zone117x opened this issue Mar 16, 2021 · 5 comments

Comments

@zone117x
Copy link

zone117x commented Mar 16, 2021

I ran into this problem while using path-to-regex in my own app, and was able to reproduce it in the unit tests in this repo.

For example, when using the route:

  app.get('/test/*', (req, res) => {
    res.sendStatus(200);
  });

The parsing error is thrown:

  ● path-to-regexp › rules › '/test/*' › encountered a declaration exception

    TypeError: Unexpected MODIFIER at 6, expected END

      155 |     if (value !== undefined) return value;
      156 |     const { type: nextType, index } = tokens[i];
    > 157 |     throw new TypeError(`Unexpected ${nextType} at ${index}, expected ${type}`);
          |           ^
      158 |   };
      159 | 
      160 |   const consumeText = (): string => {

      at mustConsume (src/index.ts:157:11)
      at parse (src/index.ts:228:5)
      at stringToRegexp (src/index.ts:495:25)
      at Object.pathToRegexp (src/index.ts:617:10)
      at Suite.<anonymous> (src/index.spec.ts:2804:33)
      at src/index.spec.ts:2802:7
          at Array.forEach (<anonymous>)
      at Suite.<anonymous> (src/index.spec.ts:2799:11)
      at Suite.<anonymous> (src/index.spec.ts:2798:3)
      at Object.<anonymous> (src/index.spec.ts:2705:1)

See Express documentation for global/wildcard route definitions here:
https://expressjs.com/en/4x/api.html#router.methods

A PR was opened here to demo the bug: #245

@blakeembrey
Copy link
Member

The wildcard is not a supported feature for the version you’re testing. Are you sure it’s the same error as express? Express uses the 0.1.x branch of code, so a test on master isn’t related. If it’s from express, have you done something to force a more recent version of this package by changing resolution somewhere?

@blakeembrey
Copy link
Member

@zone117x
Copy link
Author

Thanks for the info. I didn't see the compatibility docs. I'm using these routes in an Express v4.x app, and was expecting this library to mirror the functionality.

I see now that this wildcard route is not supported here. Curious if you have any recommendations for me -- should I use the Express v5-alpha, or avoid using route syntax that is incompatible with this lib?

Thanks for the quick response ❤️

@zone117x
Copy link
Author

zone117x commented Mar 16, 2021

Also, has Express stated this behavior is a bug, or only this library?

For example, in their docs they show this route:

router.all('/api/*', requireAuthentication)

I'd be interested in some kind of way to "opt-in" to Express v4.x compatible parsing, via a config option or otherwise. I looked into trying to support this parsing in the code for a possible PR but got lost. Any tips?

@blakeembrey
Copy link
Member

This isn't a bug, just a feature change. It might be flagged somewhere, and it's something that could be added back. Opting in to express 4.x compatible parsing probably wouldn't be possible though, there's some really iffy behavior such as regexp characters being valid anywhere in the string that would cause problems.

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