Allow more than route to be registered for the same url and method

[airhorns]

Newer fastify versions support route constraints, which means that a path & method don't uniquely identify a route anymore. Fastify will allow multiple route handlers to be added for the same path and method if they have different constraints.

For this reason, I think we should change the data structure exposed by this library to expose these routes. Before this change, this library will only report the most recently registered routes and completely omit previously registered ones.

Instead of

{
  "/foo/bar": {
      "GET": { ... },
      "POST": { ... },
   },
  "/hello": {
      "GET": { ... },
   },
}

this PR switches us to use an array of route options for each URL:

{
  "/foo/bar": [
      { ... },
      { ... },
   ],
  "/hello": [
      { ... },
   ],
}

I think this is still useful in that you can get to the routes you want quickly, but supports reporting all the routes registered and is a bit more future proof.

Also important is that there are new route options that this library omits. I think it'd be more useful to report all the route options like constraints (which is built into fastify) or websocket (from fastify-websocket). For this reason, I think we should capture the whole route options object fastify sends to the onRoute hook. That way, downstream consumers of the route map have access to the most information possible.

And finally, I think that using the raw routeOptions object from fastify allows subsequent onRoute hooks to modify the route options. Before this change, if this plugin was registered first and then a onRoute hook was registered after, changes that second hook made to the route options wouldn't be captured. Using the raw object without copying allows this plugin to reflect changes made by subsequent hooks.

This is for sure a breaking change, so if y'all are ok with this direction I will add a changelog entry to explain how to upgrade.

Checklist

• run npm run test and npm run benchmark
• tests and/or benchmarks are included
• documentation is changed or added
• commit message and code follows the Developer's Certification of Origin
  and the Code of conduct

[mcollina]

This likely require a readme change.

[airhorns]

This likely require a readme change.

Yep, will change, but are you ok with the breaking change / new approach? I will update the readme and add a changelog entry if so.

[Eomm]

Yep, will change, but are you ok with the breaking change / new approach?

I think your points are good.

Personally, I think that it would be annoying for users that do not use constraints to write: fastify.routes['/foo'][0]
So I would like

• a keepPlain flag as a plugin parameter that will not create the array but only the plain object
• or a manageConstraing flag that will produce the array for the endpoint key

[mcollina]

Another option is to use an array only if there are multiple constraints, plus an option to always have arrays.

[airhorns]

I think it's kinda weird that someone who's using constraints would have to realize and know that they needed to pass that option, and also, it means that we have to maintain two codepaths instead of one. Personally, I feel that increases overall complexity without really adding much value, because I don't feel fastify.routes['/foo'][0] is really much more annoying than fastify.routes['/foo'].get? I also think that returning arrays sometimes and not othertimes violates the principle of least surprise -- you have to know that arrays might sometimes come out of this plugin's API and if you are trying to write robust code or another general plugin, you'd have to always check if it was an array or an object. That again seems a bit more complicated than just always returning an array, but maybe I just have a different aesthetic and don't care about that array?

Also, for options like manageConstraining, it would totally work, but why not just have users do that in userland and keep this library simple and easy to understand? A use can iterate the array and do whatever they want, and I think trying to bake in helpers for data access patterns into this library is tricky because you have to anticipate all the user's needs instead of just giving them the data and letting them do whatever they want, you know?

[mcollina]

I'm happy with the change as it is. @Eomm wdyt?

[Eomm]

I totally agree with @airhorns that a mixed situation would be really annoying.

From my point of view, I think the fastify.routes['/foo'][0] is annoying because I must be aware of the output (and understand why it is an array) and then acts due to the constraint feature that a developer could not use.
I see this extra effort.

I think this feature is fine, but I think I may open an issue to add the keepPlain flag since it would be just a setting for those that would like the plain object 👍🏽

[mcollina]

lgtm

could you ship a major as well?

[mdeltito]

just a note in case it comes up for others, this ended up landing as a semver minor in v3.1.0