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.

Sign up for GitHub

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

Jump to bottom

Route schema might be undefined #5393

Closed
2 tasks done
nflaig opened this issue Apr 8, 2024 · 4 comments · Fixed by #5394
Closed
2 tasks done

Route schema might be undefined #5393

nflaig opened this issue Apr 8, 2024 · 4 comments · Fixed by #5394
Labels
bug Confirmed bug typescript TypeScript related

Comments

@nflaig
Copy link
Contributor

nflaig commented Apr 8, 2024 •
edited

Prerequisites

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

Fastify version

4.x.x

Plugin version

No response

Node.js version

20.x

Operating system

Linux

Operating system version (i.e. 20.04, 11.3, 10)

Ubuntu 22.04

Description

FastifyRequest.routeSchema might be undefined, for example if route does not exists. The type currently can't be trusted if you would try to access the schema within any hook.

I think the type here should be marked as optional

fastify/types/request.d.ts

Line 67 in 4a35a5a

routeSchema: FastifySchema

and same goes for schema in route options

fastify/types/request.d.ts

Line 34 in 4a35a5a

schema: SchemaCompiler;

Steps to Reproduce

Set up fastify with a hook like onRequest and observe schema while calling a route that has no been defined.

Expected Behavior

The type should correctly reflect runtime behavior
@nflaig nflaig mentioned this issue Apr 8, 2024
Merged
@Eomm
Copy link
Member

Eomm commented Apr 9, 2024

Thanks for reporting!
Would you like to send a Pull Request to address this issue? Remember to add unit tests.

@Eomm Eomm added bug Confirmed bug typescript TypeScript related labels Apr 9, 2024
This was referenced Apr 9, 2024
Merged
Closed
@nflaig
Copy link
Contributor Author

nflaig commented Apr 9, 2024

Thanks for reporting! Would you like to send a Pull Request to address this issue? Remember to add unit tests.

I opened 2 PRs as I wasn't sure what you'd prefer

@gurgunday
Copy link
Member

I think in next we should fix the type instead and it should be undefined again

@mcollina
Copy link
Member

mcollina commented May 6, 2024

We can do that in v4 if you all prefer to allow it to be undefined.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Confirmed bug typescript TypeScript related
Projects
None yet
Milestone
No milestone
4 participants
@mcollina @Eomm @nflaig @gurgunday