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

docs: improved migration guide for onRoute #4233

Merged
merged 2 commits into from Aug 30, 2022
Merged

docs: improved migration guide for onRoute #4233

merged 2 commits into from Aug 30, 2022

Conversation

ShogunPanda
Copy link
Contributor

@ShogunPanda ShogunPanda commented Aug 30, 2022

This PR improves the migration guide to explicit how to define routes when onRoute is also used.

@github-actions github-actions bot added the documentation Improvements or additions to documentation label Aug 30, 2022
Copy link
Member

@climba03003 climba03003 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you think it is misleading?
The solution can be chosen from either one, and now it is totally different meaning.

@ShogunPanda
Copy link
Contributor Author

Ehm, because I tried. :)
I noticed while upgrading my npm package fastify-print-routes to fastify v4.
Using fastify-plugin is recommended but it is not enough to make that working, await register is the key.

@climba03003
Copy link
Member

climba03003 commented Aug 30, 2022

wrap your routes in a plugin (recommended)

import Fastify from 'fastify'
import FP from 'fastify-plugin'

const fastify = Fastify()
fastify.register(FP(function(fastify, option, done) {
  fastify.addHook('onRoute', function() {
    console.log('onRoute')
  })
  done()
}))

fastify.register(function(fastify, option, done) {
  fastify.get('/', () => 'hello')
  done()
})

use await register(...)

import Fastify from 'fastify'
import FP from 'fastify-plugin'

const fastify = Fastify()
await fastify.register(FP(function(fastify, option, done) {
  fastify.addHook('onRoute', function() {
    console.log('onRoute')
  })
  done()
}))

fastify.get('/', () => 'hello')

@ShogunPanda
Copy link
Contributor Author

ShogunPanda commented Aug 30, 2022

Ops. It makes sense now. I'll change it so it's more explicit.

PR contents and title updated.

@ShogunPanda ShogunPanda changed the title docs: corrected misleading migration instructions. docs: improved migration guide for onRoute Aug 30, 2022
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@RafaelGSS RafaelGSS merged commit 67bee11 into fastify:main Aug 30, 2022
@ShogunPanda ShogunPanda deleted the migration-typo branch August 30, 2022 12:07
@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 31, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants