Skip to content

fix(ts): Align routerOptions defaultRoute types with runtime#6392

Merged
Eomm merged 15 commits intofastify:mainfrom
AnkanMisra:main
Jan 2, 2026
Merged

fix(ts): Align routerOptions defaultRoute types with runtime#6392
Eomm merged 15 commits intofastify:mainfrom
AnkanMisra:main

Conversation

@AnkanMisra
Copy link
Contributor

Summary

Fixes #6389.

What was broken

routerOptions.defaultRoute and routerOptions.onBadUrl were typed as receiving FastifyReply, but the runtime always provides the raw Node ServerResponse. Consumers relying on the typed FastifyReply helpers (for example reply.send) hit runtime errors despite the compiler reporting everything as safe.

What this PR changes

  • retunes the TypeScript definitions for both callbacks to RawRequestDefaultExpression/RawReplyDefaultExpression, so the compiler finally describes what the runtime actually hands you
  • calls out in docs/Reference/Server.md that these hooks get raw IncomingMessage/ServerResponse objects, setting expectations for folks writing plain JavaScript too
  • drops a tsd regression in test/types/instance.test-d.ts so routerOptions can’t quietly drift back to exposing FastifyReply

Testing evidence

  • npm run lint → passes (eslint only)
  • npm run unit → passes (borp, 2,140 tests run, 0 failures, 4 skips)
  • npm run test:typescript → passes (tsc + tsd)
  • npm run benchmark --if-present → passes (autocannon ~119k req/s avg, 672 MB read over 30 s)

Checklist

@github-actions github-actions bot added documentation Improvements or additions to documentation typescript TypeScript related labels Nov 15, 2025
@Eomm Eomm added the bugfix Issue or PR that should land as semver patch label Nov 16, 2025
@Eomm Eomm changed the title Fix : Align routerOptions defaultRoute types with runtime fix(ts): Align routerOptions defaultRoute types with runtime Nov 16, 2025
@AnkanMisra
Copy link
Contributor Author

@Eomm @gurgunday
If there's anything in this PR that may need further clarification, I will be happy to rework it

@gurgunday gurgunday requested review from a team and Uzlopak November 16, 2025 18:29
@Eomm
Copy link
Member

Eomm commented Nov 17, 2025

@Eomm @gurgunday If there's anything in this PR that may need further clarification, I will be happy to rework it

No, there was an internal discussion regarding this PR and we were thinking if we should use the find-my-way's types instead.

So waiting here some other review from the core team

@AnkanMisra
Copy link
Contributor Author

Just to be sure, should Fastify expose the full find-my-way config or only a subset of it?
I will update the types and validation once that’s clear

@Eomm
Copy link
Member

Eomm commented Nov 29, 2025

Just to be sure, should Fastify expose the full find-my-way config or only a subset of it? I will update the types and validation once that’s clear

The whole routerOptions object is propagated to find-my-way:

fastify/fastify.js

Lines 97 to 99 in 012c249

const router = buildRouting({
config: options.routerOptions
})

@AnkanMisra AnkanMisra requested a review from Eomm November 29, 2025 18:29
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.

Last suggestion, then LGTM and it seems a real fix not targetting ts only!

@AnkanMisra
Copy link
Contributor Author

AnkanMisra commented Dec 8, 2025

@Eomm

I have added a regression test in test/router-options.test.js to confirm that extra find-my-way options are supported: https://github.com/fastify/fastify/pull/6392/files#diff-9d08a4c2cefc7ab82d16586f97b29534c288d2161d98f29b36a19fbc2454fe1dR898-R

I have also verified that the full test suite and benchmarks pass locally.

@AnkanMisra AnkanMisra requested a review from Eomm December 9, 2025 15:23
@AnkanMisra AnkanMisra requested a review from Eomm December 14, 2025 17:16
@AnkanMisra
Copy link
Contributor Author

@Eomm is there any problem related to the quality of code of this pr according to fastify?

Comment on lines +17 to +28
const {
asString,
asNumber,
asBoolean,
asDateTime,
asDate,
asTime,
asUnsafeString
} = serializer

const asInteger = serializer.asInteger.bind(serializer)

Copy link
Member

Choose a reason for hiding this comment

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

Why does the diff show these?

Copy link
Member

Choose a reason for hiding this comment

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

Can you rebase properly so we only see your changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

PTAL @gurgunday

@AnkanMisra AnkanMisra requested a review from gurgunday December 29, 2025 14:14
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

@Eomm Eomm merged commit f18cda1 into fastify:main Jan 2, 2026
31 of 32 checks passed
@Eomm
Copy link
Member

Eomm commented Jan 2, 2026

Merged

Linting issue not related to this PR

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

Labels

bugfix Issue or PR that should land as semver patch documentation Improvements or additions to documentation typescript TypeScript related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Typescript] incorrect type definition for routerOptions.defaultRoute

5 participants