Typings don't allow custom query builders

[elhigu]

I'm trying to create Model, which has extended query builder with .session() method. However when I'm extending the model and setting our custom query builder there return type of $query() and other functions still is QueryBuilder<this> instead of CustomQueryBuilder<this>.

Simplified (not runnable example):

class CustomQueryBuilder<T> extends QueryBuilder<T> {
  session(session: any) {
    this.context().session = session;
    return this;
  }
}

class BaseModel extends Model {
  // Override the objection.js query builders classes for subclasses.
  static QueryBuilder = CustomQueryBuilder;
  static RelatedQueryBuilder = CustomQueryBuilder;

  // ... more stuff ...
}

const daa = await BaseModel
          .query(trx)
          // ERROR: [ts] Property 'session' does not exist on type 'QueryBuilder<BaseModel>'
          .session(req.session)
          .insert({1:1});

Any ideas how to override this? I wouldn't like to do casting everywhere I'm using my extended query builder, nor I would like to add extra methods to BaseModel which would apply correct typing for returned query builder.

[koskimas]

@mceachen Could you take a look at this?

[mceachen]

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.

[koskimas]

@elhigu Can't you just do this:

class BaseModel extends Model {
  static query(...args): CustomQueryBuilder {
    return super.query(...args) as CustomQueryBuilder;
  }

  $query(...args): CustomQueryBuilder {
    return super.$query(...args) as CustomQueryBuilder;
  }

  $relatedQuery(...args): CustomQueryBuilder {
    return super.$relatedQuery(...args) as CustomQueryBuilder;
  }
}

this example won't work but you see what I mean? Override the relevant BaseModel methods to return the correct inherited type.

[elhigu]

@koskimas I tried that, but typescript tells me that those methods doesn't have correct signature :(

But this syntax I learned recently, might work also for class methods I haven't tried it yet (https://www.typescriptlang.org/docs/handbook/functions.html):

function pickCard(x: {suit: string; card: number; }[]): number;
function pickCard(x: number): {suit: string; card: number; };
function pickCard(x): any {
    // Check to see if we're working with an object/array
    // if so, they gave us the deck and we'll pick the card
    if (typeof x == "object") {
        let pickedCard = Math.floor(Math.random() * x.length);
        return pickedCard;
    }
    // Otherwise just let them pick the card
    else if (typeof x == "number") {
        let pickedSuit = Math.floor(x / 13);
        return { suit: suits[pickedSuit], card: x % 13 };
    }
}

[tobalsgithub]

@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>

[mceachen]

@tobalsgithub's approach should work, but it'd be nice if the typings "just worked" without that.

Also, be aware that you'll only have your new methods available from chained method calls that return a this (which is about half the methods). Other methods will have reverted to the default query builders.

For example, BaseModel.query().relate() will still return a CustomQueryBuilder, but BaseModel.query().insertAndFetch() will give you a QueryBuilderSingle<BaseModel>.

The only way around this that I see is introducing both generic subtypes for Model as well as generic subtypes for QueryBuilder and that hierarchical tree.

[elhigu]

Hi, thanks everyone. I'll report back after I got a change to try out these solutions.

IMO if I get any of these working it is good enough workaround. Anyways at least for now this is quite rare use case and overriding $query etc. shouldn't be a show stopper for anyone..

[elhigu]

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

[elhigu]

This actually worked... just had to write interface declaration for QueryBuilderSingle mode of subclassed query builder.

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

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

interface BaseQueryBuilderSingle<T> extends QueryBuilderBase<T>, Promise<T> {
  session(session: any): this;
}

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

  $query(trx?: Transaction): BaseQueryBuilderSingle<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({});

[mceachen]

Once TypeScript supports generics with defaults (see the issue and pr), we may be able to capture the custom query builder signature into the Model generic, and not break the current typing API.

Edit: huh, it did get merged in 2.3, I'd missed that! I'll take another look at this when I get a chance.

[brianphillips]

@mceachen did you ever come back to looking at this and if so, have any success?

[mceachen]

I didn't. If you have time, I'd be happy to help review a PR though.

[jtlapp]

I need this fix as well and have been looking over how to do it. I have a proposal, but these typedefs are pretty complex, and I don't know if it'll work or how much reworking it requires.

Right now, as best I can make sense of things, the model class T and the query builder class QueryBuilder are both assigned via either query() or $query().

What if we instead assign them via Model's static QueryBuilder getter? That way the type of the query builder would be the type of value returned, whatever it happens to be, subject to the constraint that it extends QueryBuilder?

I'm looking into this now, but my minimal experience with Typescript type decls is making this a challenge.

UPDATE: "Accessors can't be declared in an ambient context." If only this had been a method!

OOPS: Okay, now I'm realizing that it's the call to Model.query() that yields the type assignment, so this wouldn't have worked anyway. Back to the drawing board...