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

chore: Add named hooks for improved visibility and debugging #261

Merged
merged 1 commit into from
Sep 21, 2024

Conversation

kei-ichi
Copy link
Contributor

This commit introduces named hooks in place of anonymous functions for the Fastify Helmet plugin. The changes include:

  1. Renamed the first onRequest hook to 'helmetConfigureReply'
  2. Renamed the second onRequest hook to 'helmetApplyHeaders'

These named hooks provide better visibility when inspecting routes and improve the debugging experience. This change aligns the plugin with recommendation in the Accelerate Web Development with Fastify book and other Fastify plugins like @fastify/cors that use named hooks.

The functionality remains unchanged; this is purely a developer experience improvement.

Original behavior:

---------------------------------------------------------------------------------------------------------
├── / (GET)
│   • (onRequest) ["handleCors()","anonymous()","anonymous()"]
│   • (onSend) ["addRequestIdHeaderHook()"]
│   / (HEAD)
│   • (onRequest) ["handleCors()","anonymous()","anonymous()"]
│   • (onSend) ["addRequestIdHeaderHook()","headRouteOnSendHandler()"]
└── * (OPTIONS)
    • (onRequest) ["handleCors()","anonymous()","anonymous()"]
    • (onSend) ["addRequestIdHeaderHook()"]

---------------------------------------------------------------------------------------------------------

After modified:

├── / (GET)
│   • (onRequest) ["handleCors()","helmetConfigureReply()","helmetApplyHeaders()"]
│   • (onSend) ["addRequestIdHeaderHook()"]
│   / (HEAD)
│   • (onRequest) ["handleCors()","helmetConfigureReply()","helmetApplyHeaders()"]
│   • (onSend) ["addRequestIdHeaderHook()","headRouteOnSendHandler()"]
└── * (OPTIONS)
    • (onRequest) ["handleCors()","helmetConfigureReply()","helmetApplyHeaders()"]
    • (onSend) ["addRequestIdHeaderHook()"]

Checklist

  • run npm run test and npm run benchmark (benchmark command not included in package.json file)
  • tests and/or benchmarks are included
  • documentation is changed or added documentation is intact
  • commit message and code follows the Developer's Certification of Origin
    and the Code of conduct

This commit introduces named hooks in place of anonymous functions
for the Fastify Helmet plugin. The changes include:

1. Renamed the first onRequest hook to 'helmetConfigureReply'
2. Renamed the second onRequest hook to 'helmetApplyHeaders'

These named hooks provide better visibility when inspecting routes
and improve the debugging experience. This change aligns the plugin
with other Fastify plugins like @fastify/cors that use named hooks.

The functionality remains unchanged; this is purely a developer
experience improvement.
@Uzlopak
Copy link
Contributor

Uzlopak commented Sep 21, 2024

Is there a way to write a test? Not that in few weeks somebodys comes around the corner and suggest arrow functions for less memory overhead and whatever.

@Eomm

Copy link
Member

@Eomm Eomm left a comment

Choose a reason for hiding this comment

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

Is there a way to write a test?

I would not bother with a test, I do agree that this is a DX improvement only - no side effect on sight

Copy link
Contributor

@Uzlopak Uzlopak left a comment

Choose a reason for hiding this comment

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

LGTM

@Uzlopak Uzlopak merged commit ef39992 into fastify:master Sep 21, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants