-
Notifications
You must be signed in to change notification settings - Fork 368
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
Why is pathToRegexp mutating keys parameter? #236
Comments
This is just the API that was originally designed, and I’ve avoided changing it so far. I’m not sure there’s a good reason to change it. I’ve instead moved some functionality away from raw regexes to make it a bit more user friendly. We could probably start returning |
Immutability and clarity in code would be pretty good reasons in my opinion :) I believe |
Hi! I'm new to the project but I wanted to contribute to it since I really like it. I was wondering if there is any interest in this issue since I wanted to try to pick it up. |
The examples are unclear on why it works that way.
Mutating params feels wrong, because in the code it is not very easy to understand how my empty keys array gets populated. So it's always necessary to add a comment about how it works (if I want the next developer to understand it as well).
Wouldn't it make better sense to return the keys from pathToRegexp? For example, like so:
The text was updated successfully, but these errors were encountered: