Skip to content

fix: wrong reply return type#6026

Merged
mcollina merged 1 commit intofastify:mainfrom
dangkyokhoang:fix-reply-return-type
Mar 27, 2025
Merged

fix: wrong reply return type#6026
mcollina merged 1 commit intofastify:mainfrom
dangkyokhoang:fix-reply-return-type

Conversation

@dangkyokhoang
Copy link
Copy Markdown
Contributor

Fixes #6020

Checklist

@github-actions github-actions bot added the typescript TypeScript related label Mar 24, 2025
Comment on lines +1016 to +1033
expectAssignable(server.get<{
Reply: {
200: string | { msg: string }
400: number
'5xx': { error: string }
}
}>(
'/',
async (_, res) => {
const option = 1 as 1 | 2 | 3 | 4
switch (option) {
case 1: return 'hello'
case 2: return { msg: 'hello' }
case 3: return 400
case 4: return { error: 'error' }
}
}
))
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.

Does this behave as expected?

declare const bool: boolean
expectAssignable(server.get<{
  Reply: {
    200: "hello_200"
    400: "hello_400"
  }
}>(
  '/',
  async (_, res) => {
    if (bool) {
      res.status(200)
      return "hello_200"
    }

    res.status(400)

    return "hello_400"
    
  }
))

expectError(
  server.get<{
    Reply: {
      200: "hello_200"
      400: "hello_400"
    }
  }>(
    '/',
    async (_, res) => {
      if (bool) {
        res.status(200)
        // Doesn't match declared 200 
        return "hello_400"
      }

      res.status(400)
      // Doesn't match declared 400
      return "hello_200"
    }
  )
)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure if this is possible. This PR only enables you to return 'hello_200' without type assertions return 'hello_200' as unknown as { 200: 'hello_200', 400: 'hello_400' }. The behavior aligns with that of other methods of defining return types, e.g. via typebox.

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.

Ok, so I don't understand why to allow this? You have use cases when you need to mix chained .status(statusCode).send(body) method with direct return?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ok, so I don't understand why to allow this?

Well, defining Reply with status codes is valid, then returning Reply[code] from a handler should be valid as well. Currently, if you define Reply types with json schema/typebox, you can return Reply[code] without issues. However, if you define types with RouteGeneric, you just can't return Reply[code] without semantically wrong type assertions. That's a bug.

You have use cases when you need to mix chained .status(statusCode).send(body) method with direct return?

Yes, I mix .status(code).send with return. People do that all the time. It's natural to return a value from a function, and throw or return reply.status().send() when you don't. It's just people often use fastify typescript with a type provider like typebox/zod and they don't have this problem.

I instead use some sort of a plugin/compiler to generate json schemas from the generic types and register them with fastify.

Copy link
Copy Markdown
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

Good work!

@mcollina mcollina requested a review from a team March 27, 2025 10:16
@mcollina mcollina merged commit 2c0be02 into fastify:main Mar 27, 2025
25 checks passed
@github-actions
Copy link
Copy Markdown

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 Mar 25, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

typescript TypeScript related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Invalid return type in route handler with generic reply type with status codes

3 participants