Removal of declaration merging

[mcollina]

declaration merging is the technique we use in Fastify to decorate the types of our base objects. See https://github.com/fastify/fastify-static/blob/d40ce2d33481da7abcd261b1e8699ef31a9e2854/types/index.d.ts#L8-L17.

This technique has a major flaw: once a module is "seen" by TypeScript, those types are mergerd in original definition, even if the decorating plugin is not loaded in the current plugin tree.

After studying a bit of TypeScript, I think we can do better. In essence, we should be able to support something like:

import fp from 'fastify-plugin'
import { FastifyStaticMixin } from '@fastify/static'

export default fp(async function (app: FastifyInstance & FastifyStaticMixin, opts: object) {

})

Or even better:

import fp from 'fastify-plugin'
import { FastifyStaticMixin } from '@fastify/static'

export default fp(async function (app: FastifyInstance & FastifyStaticMixin, opts: object) {

}, {
  decorators: {
    reply: FastifyStaticMixin
  }
})

Or some other analog syntax with native Fastify, e.g. withMixin<T>().

I think this would be a significant improvement on our TS story for v5. Wdyt?

[DemonHa]

Maybe we can use assertions to keep track of registered plugins, so plugins don't have to do declaration merging.

@mcollina
Proof of concept

[mcollina]

Maybe we can use assertions to keep track of registered plugins, so plugins don't have to do declaration merging.

Wow! I'm learning new magic. This could work well inside a single file.
How would that work across files? The most common form of Fastify plugin has no clue on what was registered before.

[Uzlopak]

@webstrand gave us few weeks/months ago an asssessment regarding builder api.

#4081 (comment)

[mcollina]

The builder API does not travel well across files... we need a solution that allows us to define in TS that plugin A requires plugin B and therefore those decorators are available.

[DemonHa]

@mcollina Exactly, I am working on a library which had the exact same problem. Can I add you as a contributor so you can see it in action?

[mcollina]

Absolutely.

[mcollina]

I did a quick check but I could not orient myself to see how you fixed this. Could you give me some pointers (here, in an issue there, or privately).

[DemonHa]

@mcollina How can I contact you?

[mcollina]

email, twitter dm, discord dm (there is a fastify discord)

[flakey5]

import fp from 'fastify-plugin'
import { FastifyStaticMixin } from '@fastify/static'

export default fp(async function (app: FastifyInstance & FastifyStaticMixin, opts: object) {

}, {
  decorators: {
    reply: FastifyStaticMixin
  }
})

Continuing the usage of fastify static as an example, would the app: FastifyInstance & FastifyStaticMixin and decorators object be in the fastify static codebase and globally modify the FastifyInstance and FastifyReply types after the fastify static plugin is registered, or would this code be what the user would write when they have registered fastify static and want to use reply.sendFile for example?

[mcollina]

would this code be what the user would write when they have registered fastify static and want to use reply.sendFile for example?

That (or something similar) is the code that a user would write to get the mixin. Maybe we can simplify it a bit.

[flakey5]

What would be the purpose of the decorators object?

[mcollina]

The purpose is to add the necessary mixins to the passed in FastifyInstance so that reply.sendFile exists.

[flakey5]

Been playing around the past couple days to see what I could get working:

1. Each type that's being extended needs its own mixin (e.g. for fastify static to add properties onto FastifyInstance we can make FastifyStaticMixin, and for adding properties to FastifyReply we can make FastifyStaticReplyMixin)

2. Instead of fp taking in just FastifyInstance for example, it will need to take in generic that extend those types (see Playground Link)

3. Here's where I'm at so far in regards to adding the decorators object: Playground Link. It takes in the decorator types fine but the thing I'm not sure how to do is combining the mixin to the fastify types (e.g. FastifyReply). I tried applying the approach that @DemonHa pointed out here Removal of declaration merging #5061 (comment) but wasn't able to get that working.

How would people feel about an api for using request & reply mixins that's similar to the proposed way to use mixins for FastifyInstance?
E.g.

fastify.route({
  url: '/',
  method: 'GET',
  handler: (_, reply: FastifyReply & FastifyStaticReplyMixin) {
    reply.sendFile('index.html');
  }
});

[mcollina]

1. yes
2. yes
3. no

How would people feel about an api for using request & reply mixins that's similar to the proposed way to use mixins for FastifyInstance?

Unfortunately this is not good. There can be tens of plugins registered, so this would become untenable in the route definition.
I'm ok for this to be in the plugin definition, because in this case things can be reused.

---

Note that fp is only an utility on top facilities in Fastify, so we would need to design something for pure register (if possible).

[flakey5]

I was thinking about the decorators object wrong and was able to get it partially working: Playground Link

I removed the explicit typing on the app parameter in the plugin function (see line 55, (app) => {...} instead of (app: FastifyInstance & ...) => {...}). I added a generic for the reply type on FastifyInstance so that it can pass it onto the route function. The issue now is that when you explicitly type the app parameter it overrides the type passed in and sets the generics back to the default values. See line 67 for an example of that.

I don't think there's a way to allow for explicit typing without overriding the generics. A user still could explicitly type the parameter, but they would need to pass in the exact same values for the generics to get the same type originally provided and that would completely defeat the purpose of the decorators object (imo at least). However, I was able to get an api like this working:

fp((app) => {
  app.staticSomethingSomething = false;

  app.route((reply) => {
    reply.sendFile('index.html');
  })
}, {
  decorators: {
    app: FastifyStaticMixin, // Pass in the FastifyInstance decorator here and it's applied to the app parameter
    reply: FastifyStaticReplyMixin
  }
})

Full example here Playground Link

---

Note that fp is only an utility on top facilities in Fastify, so we would need to design something for pure register (if possible).

I don't believe the approach I described above would work with register. By the time the user is registering plugins they have a FastifyInstance and we would need to figure out some way to retroactively modify the generics of that type which I don't think is possible unless we do something similar to #5061 (comment), but that still won't travel across files. We might be able to return the same instance but reinterpreted (via something like return app as unknown as FastifyInstance<...>) but I don't think that would be very useful imo.

How would you feel about using a mix of both #5061 (comment) to handle the register function to support single file apps and then the approach I described above for fastify-plugin to support multi-file apps?

[mcollina]

The playground link is ok.

An alternatvie API that we could consider is the following:

const _app = app.withAppMixin<StaticAppMixin>().withRequestMixin<StaticRequestMixin>().withReplyMixin<StaticReplyMixin>()

Wdyt of this?

[flakey5]

Wdyt of this?

Would that replace the register function adding the types to the FastifyInstance? Or would that be for calling fp(..). If it's the former I think it looks good, but I wonder if those functions would only just add the types to the FastifyInstance or if it would entirely replace the use case of register()?

[mcollina]

yes, it would replace the use case for adding this to register() and will be complimentary to the one of fastify-plugin.

[flakey5]

So the full api would be something like this?

app.ts:

const app = fastify({...})
app.register(fastifyStatic, ...)

const _app = app
  .withAppMixin<StaticAppMixin>()
  .withRequestMixin<StaticRequestMixins>()
  .withResponseMixin<StaticResponseMixin>();

// _app.route or something

some-plugin.ts:

fp((app) => {
  // app.route or something
}, {
  decorators: {
    app: StaticAppMixin,
    reply: StaticReplyMixin
  }
})

[mcollina]

I would say go ahead for the mixins. Maybe let's keep iterating on top of the plugin APIs.
Any opinion on that @DemonHa?

Something that would be rad is to have:

let app = fastify({...})
app = app.register(fastifyStatic, ...)

I guess we can start adding the Mixins and build on top of that.

[dirkdev98]

Nb. not much experience with the existing Fastify API's and types setup. Got interested in this issue while evaluating Fastify and the ecosystem :)

I think if mixins are added in the proposed way, a few extra types and generics are necessary on FastifyInstance. It should then be possible for plugins to specify their dependencies via either fp<Dependency>(...) or fp((app: FastifyPluginHelper<Dependency>) => ...), see the playground for a quick take on this.

Although a builder-like api has been discussed in #4081 I couldn't resist trying to figure out how that would work, so made a proof-of-concept over the weekend, see playground. Not sure if this concept would be completely possible to implement, haven't done much generic work in the past few years, and what the TS performance will be if everything is implemented.

[simoneb]

@ShogunPanda @marco-ippolito as discussed, ptal 🙏

[mcollina]

Quick addition: whatever solution we find needs to be able to work via JSdoc too.

[DemonHa]

I would say go ahead for the mixins. Maybe let's keep iterating on top of the plugin APIs. Any opinion on that @DemonHa?

Something that would be rad is to have:

let app = fastify({...})
app = app.register(fastifyStatic, ...)

I guess we can start adding the Mixins and build on top of that.

I'm more into this API, since with the mixins API looks like we are duplicating ourself. Also by using mixins we can fake our types, but if we integrate it on the register api, we are sure that the plugin is registered, and therefore we get the types.

[mcollina]

What we need is a set of 3 PRs that essentially go together to PoC this:

1. fastify
2. fastify-static
3. fastify-plugin

[DemonHa]

What we need is a set of 3 PRs that essentially go together to PoC this:

1. fastify
2. fastify-static
3. fastify-plugin

I already build a PoC as discussed. You can check it out on this PR.

If you navigate to examples/typescript-plugin-safety.ts and scroll to the end, you can see the API working as intended and you can play around with it.

I merged all the points on this single PR, so if we agree I can split this on specific PR's.

[flakey5]

If it's worth anything, I have a draft pr for adding mixins to fastify-static here fastify/fastify-static#427

[mcollina]

What can we do to ship this before July?

[DemonHa]

What can we do to ship this before July?

Getting feedback as early as possible would be a great help.