Add option to match only prefix root with paths#1487
Conversation
| * `logLevel`: set log level for this route. See below. | ||
| * `config`: object used to store custom configuration. | ||
| * `version`: a [semver](http://semver.org/) compatible string that defined the version of the endpoint. [Example](https://github.com/fastify/fastify/blob/versioned-routes/docs/Routes.md#version). | ||
| * `prefixRootOnly`: when using a `/` url with a prefix, setting this option to true will register only `/prefix` without trailing slash. (Defaults to `false`) |
There was a problem hiding this comment.
Not sure the option name is clear enough.
How about disablePrefixTrailingSlash? (Not sure about this one either)
There was a problem hiding this comment.
I'm open to anything, to be honest naming this option was the hardest part of the PR 😄
There was a problem hiding this comment.
Agree! I also think we should add a code example to explain the behavior of the option.
Which usually is better than thousand of words :D
There was a problem hiding this comment.
I think we really would like three options here:
- define both
/pathand/path/(default) - define
/path - define
/path/
The default is picked because http://localhost:3000/ and http://localhost:3000 are going to the same route anyway.
There was a problem hiding this comment.
I was thinking about this, perhaps a URL of / should only refer to {prefix}/ and another token for url such as . representing just the prefix?
There was a problem hiding this comment.
Unfortunately http://localhost:3000/. is a valid URL. We cannot use ..
|
What do you guys think of my latest commit? Still not sold on the naming but it's a tough one to name. I've updated the tests but I'll update the docs later. Basically default ( Thoguhts? |
| * `logLevel`: set log level for this route. See below. | ||
| * `config`: object used to store custom configuration. | ||
| * `version`: a [semver](http://semver.org/) compatible string that defined the version of the endpoint. [Example](https://github.com/fastify/fastify/blob/versioned-routes/docs/Routes.md#version). | ||
| * `prefixRootOnly`: when using a `/` url with a prefix, setting this option to true will register only `/prefix` without trailing slash. (Defaults to `false`) |
There was a problem hiding this comment.
Unfortunately http://localhost:3000/. is a valid URL. We cannot use ..
mrkstwrt
force-pushed
the
prefix-trailing-slash
branch
from
2bfc99a to
4b2844b
Compare
Eomm
left a comment
Eomm
left a comment
There was a problem hiding this comment.
Only one question: in case of prefixTrailingSlash: "no-slash", ignoreTrailingSlash: true will win the ignoreTrailingSlash?
|
@Eomm When |
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
5 participants
Since 2.0 it is not possible to target only
/v1/testwith the multiple levels of nested prefixes.The code in this sandbox would add only a single route
/v1/testin v1. However in v2 it creates two routes/v1/testand/v1/test/.This PR adds the option
prefixRootOnlywhich behaves in the old way, adding only a single route.Checklist
npm run testandnpm run benchmark