Objection.js: Discussion: TypeScript plugin/mixin woes

[kblomster]

Implementing and/or using Objection plugins in TypeScript is kinda messy, at the moment. Let's try writing a plugin in TS 2.6.1 with the current typings:

class PluginBase extends objection.Model {
  static classProp: number;
  baseProp: string;
  overridableMethod (arg: number) {
    return arg.toString();
  }
}

const plugin: objection.Plugin = (baseClass) => {
  return class extends baseClass {
    static pluginClassProp: number;
    pluginProp: string;
    overridableMethod (arg: string) {
      return Number(arg);
    }
  };
};

This doesn't work at all and causes two separate errors:

Type '<M extends typeof Model>(baseClass: M) => typeof (Anonymous class)' is not assignable to type 'Plugin'.
Type 'typeof (Anonymous class)' is not assignable to type 'M'."

```
Type 'M' is not a constructor function type.

Stealing an idea from the `Mixin` declaration, we try a different plugin function signature, and it starts looking promising:
```typescript
// note that we do _not_ explicitly declare the return type here
const plugin = <MC extends objection.ModelClass<any>>(baseClass: MC) => {
  return class extends baseClass {
    static pluginClassProp: number;
    pluginProp: string;
    overridableMethod (arg: string) {
      return Number(arg);
    }
  };
};

class PluggedIn extends plugin(PluginBase) {}

// all of this typechecks:
let instanceProp: string = new PluggedIn().pluginProp;
instanceProp = new PluggedIn().baseProp;
let classProp: number = PluggedIn.classProp;
classProp = PluggedIn.pluginClassProp;
const methodResult: number = new PluggedIn().overridableMethod('1');

But then we run into trouble anyway, because mixin() still doesn't quite work like we want:

class MixedIn extends objection.mixin(PluginBase, [plugin]) {}
// The line below doesn't compile: Property 'pluginProp' does not exist on type 'MixedIn'.
let instanceProp: string = new MixedIn().pluginProp;

And, well, of course it doesn't. The re-defined plugin example above only works because we let the compiler infer the return type for us. A plugin function does not usually return its model class argument unchanged, like the current typing implies. If we were to declare the return type of our plugin above explicitly, we'd first have to declare interfaces for both the static and the instance side of the anonymous extension class. The return type then becomes the intersection of the argument and the static interface, like so:

interface PluginInstance {
  pluginProp: string;
  baseMethod: (arg: string) => number;
}

interface PluginStatic {
  new (...args: any[]): PluginInstance; // or alternatively, have the interface extend objection.Constructor<PluginInstance>
  pluginClassProp: number;
}

const plugin = <MC extends objection.ModelClass<any>>(baseClass: MC): MC & PluginStatic => {
  // same implementation as above
}

So, fine, okay, the Plugin type in Objection's typings could look something like this instead (not sure about that Constructor<any> part though):

export interface Plugin<P extends Constructor<any>> {
  <MC extends ModelClass<any>>(modelClass: MC): P & MC;
}

And then we could declare our plugin function like this:

const plugin: objection.Plugin<PluginStatic> = (baseClass) => {
  // same implementation as before
}

And that works, at least for the case where you write a plugin in JS and want to ship a declaration file - using it from TS would work fine out of the box. If we're writing our own plugins for internal use in a TS project the Plugin type isn't really useful to us though and we can roll like in the first working example above, letting the compiler infer the intersection for us and thus avoiding the duplication of the typing information in an interface.

This only makes the mixin issue worse, though. The mixin function takes as its arguments a base type and an array of plugin functions, and its return type is an intersection of types effectively equivalent to BaseClass & Plugins[0] & Plugins[1] & (...), and it is at this point I'm starting to get a headache. I don't think you can actually express this in the TS type system in a generic way. The only way I can think of to type mixin (and compose) correctly at the moment is to hardcode an arbitrary number of fixed arity overloads and hope nobody tries to mix in more than that arbitrary number of plugins, but that makes me feel like I'm going to be featured on TheDailyWTF at any moment. If that is the only option, it might be less bad to leave it more or less like it is and let the user construct the intersection type themselves and just cast the return value to it.

What do you guys think? At the very least I think the argument to Plugin is incorrectly typed at the moment, and fixing that is trivial. Making its return type correct is also easy enough by making it a generic type. In the course of experimenting with this I already made most of a pull request for those two, but then things start getting hairy with mixin and compose and I figured I'd get your thoughts before I started cleaning that up.

[koskimas]

@mceachen

[mceachen]

I thought I'd exercised plugin and mixin sufficiently with examples.ts but I think getting an actual plugin implementation in there would help ensure the typings stay reasonable.

If you look at the git line history for mixin, you can see I've tried at least two different approaches to capture the class type--the latest iteration seemed to work for ts2.6.1 for the examples present, but I think the problem was we didn't have enough examples.

Sound good?

[kblomster]

Yeah, I think I'll make a PR with some more test cases and an attempt at making them pass and then we can get into the gory details in there.

[mceachen]

@kblomster please tag me on the pr or email me the branch when/if you want to collaborate.

[jtlapp]

If you're after a fully-typed plugin, I happened to do this last night in this temporary experimental package, porting examples/plugin. I haven't thought about making it more generic.

[brianphillips]

(caveat, I'm still pretty new to Typescript so I'm afraid I can't provide much more than aspirations)

@jtlapp a valiant effort, but in your example, seems like you have to specify type parameters explicitly for both the model and the query builder?

Person.query<Person, SessionQueryBuilder<Person>>()

I was hoping that both could be inferred automatically based on the class being used and it's type information. For instance:

Person.query() // typescript knows this is a SessionQueryBuilder<Person>

The explicit type parameters seem not much better than:

Person.query() as SessionQueryBuilder<Person>

[jtlapp]

Oh yes, absolutely. These are just typings to get us started on figuring
out a long term solution for custom QueryBuilders -- that's the whole
purpose of this package.

Even so, if you don't need to use a custom QueryBuilder, you should be able
to ignore all that, use the current typings, and model the rest of this way
of typing a plugin.

(Apologies if this response is wonky. I'm responding via email on my phone
on a train.)

On Jan 4, 2018 12:04 PM, "Brian Phillips" [e-mail] wrote:

(caveat, I'm still pretty new to Typescript so I'm afraid I can't provide
much more than aspirations)

@jtlapp https://github.com/jtlapp a valiant effort, but in your example,
seems like you have to specify type parameters explicitly for both the
model and the query builder?

Person.query>()

I was hoping that both could be inferred automatically based on the class
being used and it's type information. For instance:

Person.query() // typescript knows this is a SessionQueryBuilder

The explicit type parameters seem not much better than:

Person.query() as SessionQueryBuilder

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/Vincit/objection.js/issues/625#issuecomment-355354555,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAIbleyUgPVAPJyHiPsGo5czFKE3QQUfks5tHRKWgaJpZM4QiZ8D
.

[Wintereise]

Hi,

So how are plugins meant to be used from TypeScript /currently/?

I tried the js way, no bueno.

import * as Visibility from "objection-visibility";
export class Entity extends mixin(Model, [
    Visibility
])
{
    ....
}

This does not compile, and produces rather colorful errors.

ERROR in G:\xen\manager\src\db\entities\Entity.ts
./src/db/entities/Entity.ts
[tsl] ERROR in G:\xen\manager\src\db\entities\Entity.ts(6,5)
      TS2322: Type 'typeof import("G:/xen/manager/node_modules/objection-visibility/dist/index")' is not assignable to type 'Plugin'.
  Type 'typeof import("G:/xen/manager/node_modules/objection-visibility/dist/index")' provides no match for the signature '<M extends typeof Model>(modelClass: M): M'.

ERROR in G:\xen\manager\src\http\handlers\v1\test.ts
./src/http/handlers/v1/test.ts
[tsl] ERROR in G:\xen\manager\src\http\handlers\v1\test.ts(11,33)
      TS2339: Property 'query' does not exist on type 'typeof User'.

Any known workarounds?

[geopic]

Please can this be addressed more urgently?

Writing type declaration files for Objection mixins is impossible at the moment. I've tried adapting this into the declaration file, I've tried returning ModelClass<CustomMixinClass>, I've tried every work-around and hack that other people have shared but my IDE still throws up errors about methods and properties from the mixin not existing / being recognised.

Is it outside of TypeScript's ability right now to have an explicit declaration of the return value being an extension of the class that is passed in as an argument or is this something that can be addressed by Objection developers? I would like to know, because I don't know how else I can write the declaration files when nothing seems to work and properties / methods from the mixin that should be recognised aren't.

[javiertury]

Typescript 3.8.3 complains when a plugin is created using the Plugin interface:

import { Plugin } from 'objection';

const myPlugin: Plugin = Model => {
  // error TS2545: A mixin class must have a constructor with a single rest parameter of type 'any[]'.
  return class extends Model {}
};

I've navigated objection's issues and tried any proposal I found, but none got rid of this message. The latest typescript guide uses mixins in a different way. However the typescript 2.2 handbook describes how to apply typings to mixins which are similar to objection plugins. At least this gets rid of the error.

import { Model } from 'objection';

type Constructor<T> = new(...args: any[]) => T;

interface Plugin {
  <M extends Constructor<Model>>(modelClass: M): M;
}

const myPlugin: Plugin = Model => {
  return class extends Model {}
};

I think the problem stems from objection's Constructor<> type. It should accept ...args: any[] as input in order to allow for mixin functionality. What do you think?