Broken Typescript types when defining options and done

[StarpTech]

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

Plugin version

4.5.0

Node.js version

18

Operating system

Linux

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

Description

I can't define a plugin with options and done callback

import fp from "fastify-plugin";

export default fp(function Health(fastify, opts, done) {
  fastify.get("/health", (req, res) => {
    res.send();
  });
});

Results in TS7006: Parameter 'opts' implicitly has an 'any' type.

Steps to Reproduce

See description.

Workaround:

import fp from "fastify-plugin";
import { FastifyPluginCallback } from "fastify";

const plugin: FastifyPluginCallback = function Health(fastify, opts, done) {
  fastify.get("/health", (req, res) => {
    res.send();
  });

  done();
};

export default fp(plugin);

Expected Behavior

No response

[mcollina]

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

[StarpTech]

Sure, I think I can create a PR this weekend.

[StarpTech]

I spent some time on it. It seems the function type overloads are too dynamic and TS is not able to infer the right type, therefore fallbacks to any. Any reason why the plugin isn't implemented in TS directly? That would make the maintenance easier.

[kibertoad]

fastify ecosystem doesn't use direct TS for anything

[StarpTech]

Is it a rule? I'd rethink it.

[kibertoad]

I think @mcollina is concerned with compiled code potentially being less performant, but I have never seen any properly done research pointing out such perf degradation.

[StarpTech]

I understand it and it should be benchmarked but for those plugins like this one, it is negotiable because it's not in the hot path.

[mcollina]

Here are a few sparsed notes:

1. Fastify has a few TS modules, prominently fastify-passport is all written in TypeScript
2. TypeScript produces code that is slower than hand-crafted JS. Loading plugins is in the hot path for startup.
3. Everything that is possible by writing some code in TypeScript is possible by writing the types. I don't see how rewriting this module in TS would fix the problem.

Last but not least, the more I look at it, the more I think that any is the correct type here. What other type would
you have?

[kibertoad]

TypeScript produces code that is slower than hand-crafted JS. Loading plugins is in the hot path for startup.

Are there any benchmarks for same code that support this? All that I ever saw amounted "we wrote a roughly similar thing in TS from a scratch, and hey, it ended up being slower, but we didn't even compare emited source".
Are there any reasons why generated JS would be slower than exact same handwritten JS? Imports are likely to differ, but that's about it.

[Uzlopak]

Simple example:

https://www.typescriptlang.org/play?target=9#code/KYOwrgtgBAIglgJ2AYwC5wPYgKLmgbwFgAoKKAVQAcoBeKAcivoBoSyYMB3EWhj7lmygAZYADNUveqImDSUAEpwA5gAtJdekrWo5AXxIlkWAM4YANsAB05jMoAU8JGkw48VqgEpDxOCFTACGIAhsjAsIgo6FhQRPJUAFwMTKzy-CBJ9OlyZDKomXk5iirqmdrq+j7GICaSTlGuAPIARgBWUUn1LjF0cWSJyZRF6ZnZqbni+QyF48U6ZSW6qQbERqYW1rYOXdEgLe1oHpSeQA

[kibertoad]

@Uzlopak But you can use either of the two approaches in TS! And with second one you should get exact same perf as in JS.

[Uzlopak]

Sure. But you need this knowledge about ts to make an informed decision about which approach you want to use.

When I started with TS, I liked the fact that I just have to define the enum and have types and the runtime code covered. With the second approach, I have to define the interface and the object separately, making it double the work.

And to explain somebody that enums are bad because enums are transpiled to that does not convince an usual typescript dev.

[StarpTech]

Last but not least, the more I look at it, the more I think that any is the correct type here. What other type would
you have?

No, it's not. The arguments of both example has to work.

// Integration tests

// Here all arguments are of type any
const pluginCallbackWithOptionsIntegration = fastifyPlugin<Options>(function (fastify, options, next) {
  expectAssignable<Options>(options)
  expectAssignable<(err?: Error) => void>(next)
})
// works
const pluginAsyncWithOptionsIntegration = fastifyPlugin<Options>(async function (fastify, options) {
  expectAssignable<Options>(options)
})

Everything that is possible by writing some code in TypeScript is possible by writing the types. I don't see how rewriting this module in TS would fix the problem.

You need to express that polymorphism manually instead of letting the compiler generate the code for you. I think it can be definitely fixed but it requires significant knowledge and this prevents contributions. If the goal is to provide first-class typescript support, I'd reevaluate the approach to writing code in Typescript. I don't believe it will have significant performance drops and if, we can optimize and benchmark it.

[Uzlopak]

What you mean with polymorphism?

[StarpTech]

https://github.com/fastify/fastify-plugin/blob/master/types/plugin.d.ts#L48-L82