Skip to content

docs(examples): Fix ineffective TypeScript example #350

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+2 −1
Open

docs(examples): Fix ineffective TypeScript example #350

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
870d26d
Fix ineffective TypeScript example
smith558 Oct 29, 2025
f3fb920
Fix logging issue
smith558 Oct 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/components/QuickStart/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ fastify.listen({ port: 3000 }, (err) => {
const typescript = `import Fastify, { FastifyInstance, RouteShorthandOptions } from 'fastify'
import { Server, IncomingMessage, ServerResponse } from 'http'

const server: FastifyInstance = Fastify({})
const server: FastifyInstance<Server, IncomingMessage, ServerResponse> = Fastify({ logger: true })
Copy link

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

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

The explicit generic type parameters <Server, IncomingMessage, ServerResponse> are unnecessary in modern Fastify TypeScript usage. Fastify v4+ has improved type inference and these generics are deprecated. The simpler const server: FastifyInstance = Fastify({ logger: true }) is the recommended approach.

Copilot uses AI. Check for mistakes.
Copy link
Member

@Eomm Eomm Nov 1, 2025

Choose a reason for hiding this comment

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

IDK 😂

Copy link
Contributor Author

@smith558 smith558 Nov 1, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@Eomm I know that's the case (the types are inferred since v4+). But this code block is meant to be a trivial code example. Of which the whole purpose was just to demonstrate TypeScript typing compatibility of Fastify. This is still hugely better than it was before, which had floating unused imports appearing in a code block, with no apparent use (and breaking TypeScript lint rules). As well as the accompanying text not logically relating to the code example. Otherwise, the whole code block should just be removed completely (as it doesn't demonstrate anything and actually breaks good practices).

I am not sure how much Copilot feedback is relevant here as this is meant to be a training article. It likely misses that the point here is "demonstration".

Copy link
Contributor Author

@smith558 smith558 Nov 1, 2025

Choose a reason for hiding this comment

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

It may be useful to add a comment line // from Fastify v4 these types are inferred automatically, you may omit them

Copy link
Contributor Author

@smith558 smith558 Nov 1, 2025

Choose a reason for hiding this comment

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

@Eomm btw, Copilot is wrong here, this generic is not deprecated, it's just inferred and not necessary to type in, but the type generic is still used and is very much valid


const opts: RouteShorthandOptions = {
schema: {
Expand All @@ -155,6 +155,7 @@ const start = async () => {

const address = server.server.address()
const port = typeof address === 'string' ? address : address?.port
server.log.info(`Server listening on port ${port}`)
Copy link

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

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

Error: Unexpected token

Copilot uses AI. Check for mistakes.
Copy link
Member

@Eomm Eomm Nov 1, 2025

Choose a reason for hiding this comment

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

Suggested change
server.log.info(`Server listening on port ${port}`)
server.log.info('Server listening on port %s', port)

Copy link
Contributor Author

@smith558 smith558 Nov 1, 2025

Choose a reason for hiding this comment

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

Are you sure? All other examples in the docs prefer to use ${x} type of interpolation (e.g. https://fastify.dev/docs/latest/Guides/Getting-Started/). We should ensure consistency.


} catch (err) {
server.log.error(err)
Expand Down
Loading