objection.js: Typings don't allow custom query builders

Here you go:

// Objection types:

interface VoidCallback<T> {
  (t: T): void
}

interface Where<QB> {
  (): QB;
  (cb: VoidCallback<QB>): QB;
  // QueryBuilder<any> is correct typing here and not a workaround
  // because subqueries can be for any table (model).
  <QB2 extends QueryBuilder<any>>(qb: QB2): QB;
}

class QueryBuilder<M extends Model> extends Promise<M[]> {
  where: Where<this>;
}

interface ModelClass<M> {
  new (): M;
}

class Model {
  static query<QB extends QueryBuilder<M>, M extends Model>(this: ModelClass<M>): QB {
    return {} as QB;
  }

  $query<QB extends QueryBuilder<this>>(): QB {
    return {} as QB;
  }
}

// Your types:

class CustomQueryBuilder<M extends Model> extends QueryBuilder<M> {
  customMethod(): this {
    return this;
  }
}

class Person extends Model {
  static query<QB = CustomQueryBuilder<Person>>(): QB {
    return super.query() as any;
  }

  $query<QB = CustomQueryBuilder<Person>>(): QB {
    return super.$query() as any;
  }
}

async function main() {
  const query = Person.query().where().customMethod().where().customMethod();
  takesPersons(await query)

  const persons = await query;
  await persons[0]
    .$query()
    .customMethod()
    .where(qb => {
      qb.customMethod().where().customMethod()
    })
    .where(Person.query().customMethod())
    .customMethod()
}

function takesPersons(persons: Person[]) {

}

We can finally close this one. Fixed in v2.0 branch. Soon to be released

@pulse14 @alidcastano and everyone else who are pissed: The support for typing the custom query builders is not missing because we are lazy or stypid. It’s missing because adding typings for them is borderline impossible. Feel free to try. People have worked hard on trying to solve this problem FOR YOU, FOR FREE, ON THEIR SPARE TIME! Remember this is open source and you have paid nothing for this. If you want to do something, stop complaining and make a PR!

Good news is that it all seems to work. The bad news is that the types need to be completely rewritten.

So for anyone (most people) that didn’t read my comment diarrhea, this is how you would create a custom query builder after the types have been rewritten:

class CustomQueryBuilder<M extends Model, R = M[]> extends QueryBuilder<M, R> {
  // Unfortunately these three types need to be hand-written 
  // for each custom query builder.
  AQB: CustomQueryBuilder<M, M[]>
  SQB: CustomQueryBuilder<M, M>
  NQB: CustomQueryBuilder<M, number>

  customMethod(): this {
    return this;
  }
}

class BaseModel extends Model {
  static QueryBuilder = CustomQueryBuilder;
  // Unfortunately this is needed in addition to the `QueryBuilder` property.
  QB: CustomQueryBuilder<this>
}

class Person extends BaseModel {
  firstName: string;
  lastName: string;
}

The custom query builder type is correctly inherited over any method call, callback, subquery, etc. With the new types you even get autocomplete for properties when you start to type stuff like .where('. I think the new types will be much cleaner too since we don’t need to drag around three template arguments. Instead, we only drag around one, the query builder type.

@falkenhawk What’s with the eyes? 😄

God Bless you, JTL. I think it would be fun to hang out with you. 🍻I didn’t read any of this but it’s great

I just can’t agree with that perspective, for a number of reasons. I have three main points that I’ll list to keep it succinct:

1. those expectations are extremely high for basically any project in existence, and yet this is open source. When something isn’t absolutely mission-critical with billions of dollars involved, it very rarely has that level of scrutiny.
2. typescript doesn’t have enough capability to handle some of the typing structures needed that are well suited for ORMs (i don’t agree with abandoning patterns in javascript that are a good fit, because the typing system can’t handle it, especially when the types can be “good enough”)
3. Personally, I’d much rather have a solid ORM which is good at run-time, and I can be confident about that aspect, rather than focusing on the types as much. Fully typed libraries are great and i’d love an ORM to exist that you describe, but I came from TypeORM; it has more detailed types, but it’s also a giant mess with some rather large runtime bugs that are completely unnacceptable for me.

As a result of those three, that’s why I’m putting effort into helping with Objections types primarily - I want to get them to a state where people can be confident about their correctness/robustness, as much as is possible with what typescript offers. My goal is to have all of the happy path covered correctly; anything not in the happy path I’m trying to get as much as is possible handled by types as well. Things like what functions can be called, at what time, on the query builder are not something that can be accounted for at this time, for example.

I finally dipped my toes in typescript world. I was able to create this POC of custom query builders. Wouldn’t it be possible to use something like this to be able to type custom query builders? Tell me what I’m missing here @elhigu @mordof @mceachen @jtlapp? This does compile:

// Objection types:

class QueryBuilder<M extends Model> extends Promise<M[]> {
  where(): this {
    return this;
  }
}

interface ModelClass<M> {
  new (): M;
}

class Model {
  static query<QB extends QueryBuilder<M>, M extends Model>(this: ModelClass<M>): QB {
    return {} as QB;
  }
}

// Your types:

class CustomQueryBuilder<M extends Model> extends QueryBuilder<M> {
  customMethod(): this {
    return this;
  }
}

class Person extends Model {
  static query<QB = CustomQueryBuilder<Person>>(): QB {
    return super.query() as any;
  }
}

async function main() {
  const query = Person.query().where().customMethod().where().customMethod();
  takesPersons(await query)
}

function takesPersons(persons: Person[]) {

}

This requires you to override the static query method for all model classes though. and I believe we need to any cast for super.query(). Is that too ugly?

@tobalsgithub I still cannot get this stuff working, but made some progress and static method overriding seem to work already. $query is still failing, because class is extended from incompatible promise stuff or something like that. I made test project and when its working I’ll add it to greenkeeper (or merge to objection codebase) to keep example working when typings are updated (https://github.com/elhigu/objection-overriding-querybuilder-typescript).

import { Model, QueryBuilder, QueryBuilderSingle, Transaction } from 'objection';

class BaseQueryBuilder<T> extends QueryBuilder<T> {
  session(session: any) {
    return this.mergeContext({ session });
  }
}

// try to override $query() method to return correct query builder
class BaseModelDollarQueryOverrideTest extends Model {
  static QueryBuilder = BaseQueryBuilder;
  static RelatedQueryBuilder = BaseQueryBuilder;

  $query(trx?: Transaction): BaseQueryBuilder<this> {
    return <any> this.$query(trx);
  }
}

// try to override static query() method to return correct query builder
class BaseModelStaticQueryOverrideTest extends Model {
  static QueryBuilder = BaseQueryBuilder;
  static RelatedQueryBuilder = BaseQueryBuilder;

  static query<T>(trx?: Transaction): BaseQueryBuilder<T> {
    return <any> super.query(trx);
  }
}

const testDollar = new BaseModelDollarQueryOverrideTest();
testDollar.$query().session({});

BaseModelStaticQueryOverrideTest.query().session({});

Current error is:

Mikaels-MacBook-Pro-2:objection-subclassing mikaelle$ npm test

> objection-subclassing@1.0.0 test /Users/mikaelle/Projects/Vincit/objection-subclassing
> tsc

test.ts(10,7): error TS2415: Class 'BaseModelDollarQueryOverrideTest' incorrectly extends base class 'Model'.
  Types of property '$query' are incompatible.
    Type '(trx?: Transaction) => BaseQueryBuilder<this>' is not assignable to type '(trx?: Transaction) => QueryBuilderSingle<this>'.
      Type 'BaseQueryBuilder<this>' is not assignable to type 'QueryBuilderSingle<this>'.
        Types of property 'then' are incompatible.
          Type '<TResult1 = this[], TResult2 = never>(onfulfilled?: (value: this[]) => TResult1 | PromiseLike<TRe...' is not assignable to type '<TResult1 = this, TResult2 = never>(onfulfilled?: (value: this) => TResult1 | PromiseLike<TResult...'.
            Types of parameters 'onfulfilled' and 'onfulfilled' are incompatible.
              Types of parameters 'value' and 'value' are incompatible.
                Type 'this[]' is not assignable to type 'this'.
npm ERR! Test failed.  See above for more details.

I’ve got some experiments planned should either lead me to the right solution or else send me back here a bit smarter to help.

@jtlapp ooh ok. That part has viable types now 🙂 it’s in the new-typings branch, and is being worked on get those covering everything (at minimum) that was previously typed as well.

Once that process is done, I’m going to be reviewing absolutely all of the types and ensuring they’re as complete/compatible as they can be. If there’s viable changes to certain areas (e.g. some helpers to define properties on a model) for the actual js that can help improve/solidify the types, without taking away from the core style of objection, i’ll be recommending/making PRs for those as well.

@willsoto The tests need to pass before it can be merged into 2.0

I think I just woke up with a solution 😄 At least for the changing return types, but I think this could be the key for the whole problem.

interface CallbackVoid<T> {
  (arg: T): void;
}

interface RawBuilder {
  as(): this;
}

interface LiteralBuilder {
  castText(): this;
}

interface ReferenceBuilder {
  castText(): this;
}

function raw(sql: string, ...bindings: any): RawBuilder {
  return notImplemented();
}

type AnyQueryBuilder = QueryBuilder<any, any>;
type Raw = RawBuilder;
type Operator = string;
type NonLiteralValue = Raw | ReferenceBuilder | LiteralBuilder | AnyQueryBuilder;
type ColumnRef = string | Raw | ReferenceBuilder;

type Value =
  | NonLiteralValue
  | string
  | number
  | boolean
  | Date
  | string[]
  | number[]
  | boolean[]
  | Date[]
  | null
  | Buffer;

/**
 * Type for keys of non-function properties of T.
 */
type NonFunctionPropertyNames<T> = { [K in keyof T]: T[K] extends Function ? never : K }[keyof T];

/**
 * Given a model type, returns the equivalent POJO type.
 */
type Pojo<T> = { [K in NonFunctionPropertyNames<T>]: T[K] | NonLiteralValue };

/**
 * Just like Pojo<M> but all properties are optional.
 */
type PartialPojo<M> = Partial<Pojo<M>> & object;

/**
 * Extracts the model type from a query builder type QB.
 */
type ModelType<QB> = QB extends QueryBuilder<infer M> ? M : never;

/**
 * Extracts the property names of the query builder's model class.
 */
type ModelProps<QB> = Exclude<NonFunctionPropertyNames<ModelType<QB>>, 'QB'>;

/**
 * Gets the single item query builder type for a query builder.
 */
type SQB<QB extends AnyQueryBuilder> = QB['SQB'];

/**
 * Gets the multi-item query builder type for a query builder.
 */
type AQB<QB extends AnyQueryBuilder> = QB['AQB'];

/**
 * Gets the number query builder type for a query builder.
 */
type NQB<QB extends AnyQueryBuilder> = QB['NQB'];

interface WhereMethod<QB extends AnyQueryBuilder> {
  // These must come first so that we get autocomplete.
  (col: ModelProps<QB>, op: Operator, value: Value): QB;
  (col: ModelProps<QB>, value: Value): QB;

  (col: ColumnRef, op: Operator, value: Value): QB;
  (col: ColumnRef, value: Value): QB;

  (condition: boolean): QB;
  (cb: CallbackVoid<QB>): QB;
  (raw: Raw): QB;
  <QBA extends AnyQueryBuilder>(qb: QBA): QB;

  (obj: PartialPojo<ModelType<QB>>): QB;
  // We must allow any keys in the object. The previous type
  // is kind of useless, but maybe one day vscode and other
  // tools can autocomplete using it.
  (obj: object): QB;
}

interface WhereRawMethod<QB extends AnyQueryBuilder> {
  (sql: string, ...bindings: any): QB;
}

interface WhereWrappedMethod<QB extends AnyQueryBuilder> {
  (cb: CallbackVoid<QB>): QB;
}

interface WhereExistsMethod<QB extends AnyQueryBuilder> {
  (cb: CallbackVoid<QB>): QB;
  (raw: Raw): QB;
  <QBA extends AnyQueryBuilder>(qb: QBA): QB;
}

interface WhereInMethod<QB extends AnyQueryBuilder> {
  (col: ColumnRef | ColumnRef[], value: Value[]): QB;
  (col: ColumnRef | ColumnRef[], cb: CallbackVoid<QB>): QB;
  (col: ColumnRef | ColumnRef[], qb: AnyQueryBuilder): QB;
}

interface FindByIdMethod<QB extends AnyQueryBuilder> {
  (id: number): SQB<QB>;
}

class QueryBuilder<M extends Model, R = M[]> extends Promise<R> {
  AQB: QueryBuilder<M, M[]>;
  SQB: QueryBuilder<M, M>;
  NQB: QueryBuilder<M, number>;

  where: WhereMethod<this>;
  andWhere: WhereMethod<this>;
  orWhere: WhereMethod<this>;
  whereNot: WhereMethod<this>;
  andWhereNot: WhereMethod<this>;
  orWhereNot: WhereMethod<this>;

  whereRaw: WhereRawMethod<this>;
  orWhereRaw: WhereRawMethod<this>;
  andWhereRaw: WhereRawMethod<this>;

  whereWrapped: WhereWrappedMethod<this>;
  havingWrapped: WhereWrappedMethod<this>;

  whereExists: WhereExistsMethod<this>;
  orWhereExists: WhereExistsMethod<this>;
  whereNotExists: WhereExistsMethod<this>;
  orWhereNotExists: WhereExistsMethod<this>;

  whereIn: WhereInMethod<this>;
  orWhereIn: WhereInMethod<this>;
  whereNotIn: WhereInMethod<this>;
  orWhereNotIn: WhereInMethod<this>;

  findById: FindByIdMethod<this>;
}

interface StaticQueryMethod {
  <M extends Model>(this: ModelClass<M>): AQB<M['QB']>;
}

interface QueryMethod {
  <M extends Model>(this: M): SQB<M['QB']>;
}

interface ModelClass<M> {
  new (): M;
}

class Model {
  QB: QueryBuilder<this, this[]>;

  static query: StaticQueryMethod;
  $query: QueryMethod;
}

// Your types:

class CustomQueryBuilder<M extends Model, R = M[]> extends QueryBuilder<M, R> {
  AQB: CustomQueryBuilder<M, M[]>;
  SQB: CustomQueryBuilder<M, M>;
  NQB: CustomQueryBuilder<M, number>;

  customMethod(): this {
    return notImplemented();
  }
}

class BaseModel extends Model {
  QB: CustomQueryBuilder<this>
}

class Person extends Model {
  firstName: string;
}

class Animal extends BaseModel {
  name: string;
  species: string;
}

function notImplemented<T>(): T {
  return {} as any;
}

async function testWhereTypes() {
  type QB = QueryBuilder<Person, Person[]>;

  takes<QB>(
    Person.query().where('firstName', 'Sami'),
    Person.query().where('age', '>', 10),
    Person.query().where(Person.query()),
    Person.query().where(Animal.query()),
    Person.query().where(true),
    Person.query().where({ a: 1 }),
    Person.query().where(raw('? = ?', [1, '1'])),

    Person.query().where(qb => {
      takes<QB>(qb);
      qb.whereIn('id', [1, 2, 3]);
    }),

    Person.query().where(function() {
      takes<QB>(this);
      this.whereIn('id', [1, 2, 3]);
    })
  );

  const persons = await Person.query().where('id', 1);
  takes<Person[]>(persons);
}

async function custom() {
  const query = Animal.query()
    .customMethod()
    .where(
      Animal.query()
        .where({ name: 'a' })
        .customMethod()
    )
    .where(qb => {
      qb.customMethod().where('name', 'Sami');
    });

  const animals = await query;
  console.log(animals[0].name);

  const animal = await animals[0]
    .$query()
    .customMethod()
    .where(qb => {
      qb.customMethod()
        .where('a', 1)
        .customMethod();
    })
    .where(Animal.query().customMethod())
    .customMethod();

  console.log(animal.name);
}

function takes<T>(...value: T[]): T[] {
  return value;
}

So the key is to add those three subtypes MQB, SQB and NQB for the custom query builder and the QB type for each model, and that’s it! See Animal and CustomQueryBuilder in the example above.

@koskimas Would you consider into 2.0? It very much sounds like the problem won’t be solved without involving an API break (or at least without restructuring how the types of related things work).

Alternatively, could you propose a recommended way to inject custom methods (like custom where<whatever>) into query builders - our current codebase now has a tens of “as any” typecasts scattered around the codebase and our initial attempt to cleanup code with custom query methods is losing its original purpose.

Would love to help on this one (within the limits that own startup + small kids permits), but probably clear guidelines on how to solve this would help me forward.

When I first looked at Objection, I assumed that I needed a custom query builder. I put a lot of time into trying to find a way to make it work with Typescript, but I’m glad that effort ultimately failed, because it turns out that I didn’t need a custom query builder. The .context() method provided everything I needed. (Unfortunately, I’ve been off on a whole 'nother project for a while, but I mean to get back.)

@newhouse ask him about spiders

I opted to create a working Typescript version of @koskimas’s custom QueryBuilder found in examples/plugin. The README should get you started.

It was quite a bear to carry forward the generality of the plugin, but I finally succeeded.

@elhigu I was able to get the static query function definition to work. Try changing the example given by @koskimas to

class BaseModel extends Model {
  static query(trx? : Transaction): CustomQueryBuilder {
    return super.query.call(this, trx) as CustomQueryBuilder;
  }
}

Haven’t played with it too much, but the typescript does compile at least.

Haven’t figured out the $query or $relatedQuery functions yet, as those seem to require a different type of QueryBuilderSingle<this>

Sure.

So, I’m pretty sure in order to expose an extension of a QueryBuilder in your model class, we’ll need to extend Model with a generic type, something like:

export class Model<QB extends QueryBuilder>

The problem is that the QueryBuilder signature already has a generic typing associated to it, that points back to the defining Model, and I can’t use a this keyword in the generic definition, like this:

export class Model<QB extends QueryBuilder<this>>

because

[ts] A 'this' type is available only in a non-static member of a class or interface.

I’m trying to think of a clever workaround here, but if anyone comes up with a solution, I’m receptive.