Skip to content

types!: remove declaration-merging reliance with registration-scoped decorator typing#6581

Open
mcollina wants to merge 8 commits intomainfrom
feat/typed-decorators
Open

types!: remove declaration-merging reliance with registration-scoped decorator typing#6581
mcollina wants to merge 8 commits intomainfrom
feat/typed-decorators

Conversation

@mcollina
Copy link
Copy Markdown
Member

Summary

This PR removes reliance on global declaration-merging assumptions for Fastify decorator typing and moves to a registration-scoped model where type visibility follows encapsulation and register() flow.

It includes:

  • core type changes so decorate(), decorateRequest(), decorateReply() and register() carry decorator effects through instance types;
  • register option inference improvements from plugin signatures;
  • TypeScript documentation refresh for Fastify 6;
  • a new migration guide for both plugin maintainers and end-user applications:
    • docs/Guides/Migration-Guide-Declaration-Merging.md.

Context / previous attempts

This work incorporates lessons from prior attempts and follow-ups discussed in:

and provides a consolidated migration path plus updated docs.

Breaking change

This is a semver-major TypeScript change:

  • decorators no longer appear globally just because plugin types are in scope;
  • decorator visibility now follows registration and encapsulation boundaries.

Runtime encapsulation behavior is unchanged; this is a type-transport correctness change.

Validation

  • npm run test:typescript
  • npm run lint:markdown -- docs/Guides/Migration-Guide-Declaration-Merging.md docs/Guides/Index.md docs/Reference/TypeScript.md docs/Reference/Hooks.md

Issue

Closes #5061.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

> [Pino documentation](https://getpino.io/#/docs/api?id=opt-serializers) for more

This line needs to be modified, otherwise the CI will fail with the error "Fragment not found"

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed in 3448dc9 by switching this to the current Pino docs anchor.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Rebased the branch onto current main; the fix is now on top in 4e16eb9b/759868e7.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Fastify uses [Pino](https://getpino.io/#/) logging library under the hood. Since

idem here, this line needs to be modified, otherwise the CI will fail with the error "Fragment not found"

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed in 3448dc9 by using the canonical getpino.io URL without the broken fragment.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Rebased the branch onto current main; the fix is now on top in 4e16eb9b/759868e7.

@mcollina mcollina force-pushed the feat/typed-decorators branch from 1169710 to 759868e Compare March 24, 2026 09:26
@mcollina
Copy link
Copy Markdown
Member Author

Fixed the two broken Pino links and rebased the branch onto current main to clear conflicts.

Local checks run successfully:

  • npm run lint
  • npm run test:typescript
  • npx --yes linkinator docs/Guides/Index.md docs/Guides/Migration-Guide-Declaration-Merging.md docs/Reference/Hooks.md docs/Reference/Logging.md docs/Reference/Server.md docs/Reference/TypeScript.md --markdown --recurse=false --retry --redirects error --verbosity WARNING

@mcollina mcollina requested a review from Tony133 March 24, 2026 09:38
Copy link
Copy Markdown
Member

@Tony133 Tony133 left a comment

Choose a reason for hiding this comment

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

I would do a final check after merging this PR: #6605, so let's see on Typescript 6 if it doesn't give an errors on the types, for the now is LGTM

Copy link
Copy Markdown
Member

@gurgunday gurgunday 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 reason why hooks weren't updated?

GPT also had a finding about the error handler type which looks valid to me

otherwise lgtm

Comment on lines +76 to +77
request: FastifyRequest<RouteGeneric, RawServer, RawRequest, NoInfer<SchemaCompiler>, TypeProvider, ContextConfig, Logger, Decorators>,
reply: FastifyReply<RouteGeneric, RawServer, RawRequest, RawReply, ContextConfig, NoInfer<SchemaCompiler>, TypeProvider, Decorators>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This looks like the wrong decorator generic is being passed into route-level errorHandler. request and reply should receive Decorators['request'] and Decorators['reply'] no?

@gurgunday
Copy link
Copy Markdown
Member

gurgunday commented Mar 24, 2026

This is a regression test from my review that currently fails but should pass:

import fastify from '../../fastify'
import { expectType } from 'tsd'

const app = fastify()
  .decorate('svc', { ok: true })
  .decorateRequest('user')
  .decorateReply('sendOk', function () {
    return this.send({ ok: true })
  })

app.addHook('preHandler', function (req, reply, done) {
  expectType<boolean>(this.svc.ok)
  expectType<void>(req.user)
  reply.sendOk()
  done()
})

app.get('/', {
  errorHandler (error, req, reply) {
    expectType<boolean>(this.svc.ok)
    expectType<void>(req.user)
    reply.sendOk()
    reply.send({ ok: true })
  }
}, async function handler (req, reply) {
  expectType<boolean>(this.svc.ok)
  expectType<void>(req.user)
  reply.sendOk()
})

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation semver-major Issue or PR that should land as semver major typescript TypeScript related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Removal of declaration merging

3 participants