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
Add an option to disable case-sensitive routing #84
Conversation
6 similar comments
@mcollina this looks perfect, could just do with a note maybe in the README? |
Done. |
README.md
Outdated
@@ -75,6 +75,19 @@ const router = require('find-my-way')({ | |||
allowUnsafeRegex: true | |||
}) | |||
``` | |||
|
|||
According to [RFC3986](https://tools.ietf.org/html/rfc3986#section-6.2.2.1find-my-way), Fastify is case sensitive by default. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Think the anchor is wrong, includes find-my-way after the section number
index.js
Outdated
@@ -308,6 +320,9 @@ Router.prototype.lookup = function lookup (req, res) { | |||
} | |||
|
|||
Router.prototype.find = function find (method, path, version) { | |||
if (!this.caseSensitive) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (this.caseSensitive === false)?
It should make V8 happier (but I'm not super sure because the value is already a Boolean).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add also a test where you check what happens if the user registers a parameter with an uppercase letter?
Eg: /test/:Param
@delvedor updated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
See fastify/fastify#1007.
@thearegee would this work for you?