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

Support constraints when registering plugins #3615

Closed
2 tasks done
ghost opened this issue Jan 12, 2022 · 13 comments · Fixed by nearform/fastify-constraints#1 or #4428
Closed
2 tasks done

Support constraints when registering plugins #3615

ghost opened this issue Jan 12, 2022 · 13 comments · Fixed by nearform/fastify-constraints#1 or #4428
Labels
plugin suggestion Ideas for new plugins

Comments

@ghost
Copy link

ghost commented Jan 12, 2022

Prerequisites

  • I have written a descriptive issue title
  • I have searched existing issues to ensure the feature has not already been requested

🚀 Feature Proposal

When registering a plugin, you can currently pass a prefix that apply to all routes in the plugin. It'd be awesome to have that same functionality, but with a version or host constraint instead of a path prefix. I don't believe there is a simple way to do this at the moment without manually passing through options to every route, but I'd love to be wrong!

Motivation

We have a few subdomain specific APIs within our app, so rather than manually passing through options to the plugin and then passing them into each route within the plugin, it'd be really nice to simply set a constraint when registering the plugin itself and have all the routes within the plugin inherit it.

Example

fastify.register(require('./routes/v1/users'), { prefix: '/v1' }) // currently supported
fastify.register(require('./routes/v1/users'), { constraints: { host: 'auth.fastify.io' } }) // not supported
fastify.register(require('./routes/apple'), { constraints: { host: 'apple.fastify.io' } })
fastify.register(require('./routes/berry'), { constraints: { host: 'berry.fastify.io' } })
GET https://apple.fastify.io/
  200 => I LOVE APPLES

GET https://berry.fastify.io/
  200 => I LOVE BERRIES
@mcollina
Copy link
Member

This would be awesome to add. Would you like to send a PR?

@mcollina mcollina added enhancement feature request New feature to be added labels Jan 12, 2022
@ghost
Copy link
Author

ghost commented Jan 12, 2022

I'm slammed this month, but I can definitely take a stab hopefully sometime early February. Do you have any pointers or guidance on where or how it should be implemented? Would it work similar to how the prefixes work, or do you think it would be different? Or would we want to implement it in a more generic way so you could actually pass any route options during register versus specific ones (I don't know if that would actually make sense or even work :P)?

@Eomm
Copy link
Member

Eomm commented Jan 12, 2022

This is already doable within a plugin that listens for the onRoute hook and add the constraints property when necessary.

fastify.register(function plugin (instance, opts, next) {
  instance.addHook('onRoute', function hook (routeOptions) {
    routeOptions.constraints = { host: 'apple.fastify.io' }
    instance.register(require('./routes/apple'))
  })
  next()
})

fastify.register(function plugin (instance, opts, next) {
  instance.addHook('onRoute', function hook (routeOptions) {
    routeOptions.constraints = { host: 'berry.fastify.io' }
    instance.register(require('./routes/berry'))
  })
  next()
})

I have always been afraid to add new fastify's private field names to the plugin's options object to avoid collisions.

https://www.fastify.io/docs/latest/Reference/Plugins/#plugin-options

@ghost
Copy link
Author

ghost commented Jan 13, 2022

@Eomm Very interesting! I've actually never used the onRoute hook before, so never even occurred to me that was possible. Would this be seen as the "official" or recommended way of doing it, or is this still slightly a hack? I think it'd still be a cool feature to have for simplicity sake, but this is also still lightweight / easy to understand, so I could also see leaving this in userland and not adding to core. Thanks :)

EDIT: To clarify, does the register for the route also need to happen within the hook, or can it happen outside but within the same scope? e.g.

fastify.register(async function plugin (instance) {
  instance.addHook('onRoute', function hook (routeOptions) {
    routeOptions.constraints = { host: 'berry.fastify.io' }
  })
  instance.register(require('./routes/berry'))
})

@mcollina
Copy link
Member

Good spot! All of this could a also be automated with a combo of onRegister and onRoute. It looks like it is a good plugin to be developed.

@mcollina mcollina added plugin suggestion Ideas for new plugins and removed enhancement feature request New feature to be added labels Jan 13, 2022
@Eomm
Copy link
Member

Eomm commented Jan 13, 2022

is this still slightly a hack?

Not at all: it is the infinite possibilities fastify offers

EDIT: To clarify, does the register for the route also need to happen within the hook, or can it happen outside but within the same scope? e.g.

Yes, it must be the same scope (using the fastify-plugin module if necessary)

@cesarvspr
Copy link
Contributor

Are we sure that we need a new plugin for this? So this means new repo right?
How can we move forward with this? I am willing to help
@climba03003

@climba03003
Copy link
Member

Are we sure that we need a new plugin for this?

Yes

So this means new repo right?

Yes, most likely this would be a community plugin.

How can we move forward with this?

Create the plugin, submit PR for adding it as community plugin.

@Ceres6
Copy link
Contributor

Ceres6 commented Nov 16, 2022

Hi!

I'm currently working on creating the mentioned plugin and will submit a PR to add it to community plugins as soon as it's working

@simoneb
Copy link
Contributor

simoneb commented Nov 17, 2022

@Eomm @climba03003 you may want to take a look at the implementation we're putting together here nearform/fastify-constraints#1

@Ceres6
Copy link
Contributor

Ceres6 commented Nov 18, 2022

Hi! We have released v1.0.0 of nearform/fastify-constraints. Maybe you can check it to decide if it resolves this issue.

@climba03003
Copy link
Member

@Ceres6
Next step should be send a PR here and update https://github.com/fastify/fastify/blob/main/docs/Guides/Ecosystem.md

@Ceres6
Copy link
Contributor

Ceres6 commented Nov 18, 2022

@climba03003
Done!

@climba03003 climba03003 linked a pull request Nov 18, 2022 that will close this issue
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plugin suggestion Ideas for new plugins
Projects
None yet
6 participants