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.

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

Wrong types for FastifyRegister with custom logger #4435

Closed
2 tasks done
marcoreni opened this issue Nov 21, 2022 · 6 comments · Fixed by #4436
Closed
2 tasks done

Wrong types for FastifyRegister with custom logger #4435

marcoreni opened this issue Nov 21, 2022 · 6 comments · Fixed by #4436
Labels
bug Confirmed bug typescript TypeScript related

Comments

@marcoreni
Copy link
Contributor

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.10.0

Plugin version

No response

Node.js version

18.12.0

Operating system

macOS

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

12.6.1

Description

We're using a custom logger (based on Pino) in our implementation. When using withTypeProvider, we're having some issues with Typescript and the registration of plugins / routes.

If I understand correctly, FastifyRegister does not use the generic for the logger, hence we're not able to pass the custom instance inside the register function.

image

Steps to Reproduce

import { TypeBoxTypeProvider } from "@fastify/type-provider-typebox";
import { fastify } from "fastify";
import { pino } from "pino";

const server = fastify({
	logger: pino(),
}).withTypeProvider<TypeBoxTypeProvider>();

type AppFastifyInstance = typeof server;

const routes = async (instance: AppFastifyInstance): Promise<void> => {
	instance.get("/bar", {}, async (res, reply) => {
		return reply.send("ok");
	});
};

/** "routes" goes in error */
await server.register(routes, {
	prefix: "/foo",
});

Expected Behavior

It should work :)

@marcoreni marcoreni changed the title Wrong types for FastifyPlugin with custom logger and TypeBoxProvider Wrong types for FastifyRegister with custom logger and TypeBoxProvider Nov 21, 2022
@RafaelGSS
Copy link
Member

Good catch! Would you like to send a Pull Request to address this issue? Remember to add unit tests.

@jagu-sayan
Copy link

Same bug for me with almost all fastify plugins (sensible, formbody, compress, secure-session).
I don't have this bug with @fastify/static and @fastify/cors. See fastify/fastify-cors#223 (comment).

Workaround

import fastifySensible from "@fastify/sensible";
import type { SensibleOptions } from "@fastify/sensible";

app.register(
  fastifySensible as unknown as FastifyPluginCallback<SensibleOptions>,
  {},
);

@sinclairzx81
Copy link
Contributor

@marcoreni Hi.

The issue may be related to server.register() as it's possible to replicate the issue without configuring a TypeProvider.

TypeScript Link Here

import { fastify } from "fastify";
import { pino } from "pino";

const server = fastify({ logger: pino() }) // <-- no configuration

const routes = async (instance: typeof server): Promise<void> => {
  instance.get("/bar", {}, async (res, reply) => {
    return reply.send("ok");
  });
}

await server.register(routes, { // <- error on routes
  prefix: "/foo",
});

However, it's possible to write your example code this way which mitigates type errors on .register().

TypeScript Link Here

import { TypeBoxTypeProvider } from "@fastify/type-provider-typebox";
import { fastify, FastifyInstance } from "fastify";
import { pino } from "pino";

// Configure the server to use the type provider.
const server = fastify({ logger: pino() }).withTypeProvider<TypeBoxTypeProvider>()

// Ensure instance is typed as FastifyInstance
const routes = async (instance: FastifyInstance) => {

  // Optional: Re-configure type provider within the plugin
  const configured = instance.withTypeProvider<TypeBoxTypeProvider>()

  // Define routes.
  configured.get("/bar", {}, (res, reply) => reply.send("ok"));
}

// Register plugin
await server.register(routes, { prefix: "/foo" }) // ok

Hope this helps!
S

@marcoreni
Copy link
Contributor Author

marcoreni commented Nov 22, 2022

@jagu-sayan , I think what you're encountering is a different bug. If I'm right, it's discussed here: #4241 .

@sinclairzx81 you're right, it's not related to TypeProvider per se. If I use your workaround I'll lose the TypeBox Provider type enhancements, since instance will be treated as the default FastifyInstance. Declaring the typeProvider on each plugin (I'm using plugins to declare routes in separate files) may not be the best solution. Anyway, I opened a PR to fix the problem.

@RafaelGSS RafaelGSS changed the title Wrong types for FastifyRegister with custom logger and TypeBoxProvider Wrong types for FastifyRegister with custom logger Nov 22, 2022
@jagu-sayan
Copy link

Thanks @marcoreni
I do an import { fastify } from "fastify";, but I have "moduleResolution": "NodeNext" in my tsconfig.json, if I put "moduleResolution": "Node", my problem is reolved.

@Uzlopak
Copy link
Contributor

Uzlopak commented Nov 23, 2022

We are currently fixing nodenext issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Confirmed bug typescript TypeScript related
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants