Typings: allow $relatedQuery to be strongly typed without a cast

[mceachen]

This diff adds an override to $relatedQuery such that if your Model subclass to has the field defined, the result type of the QueryResult can be inferred.

Prior code that uses a cast will still work.

Related: question 2 at #708 (comment)

[coveralls]

Coverage remained the same at 95.149% when pulling a4644a7 on mceachen:keyof_related into a159e16 on Vincit:master.

[jtlapp]

Dang. You are a master of Typescript type definitions. I don't even understand what you just did, will study it hoping to learn something. Thanks!

[mceachen]

@jtlapp : The K extends keyof this requires the relation to be a field name, and this[K] embodies the type of that field.

By putting this new definition first, it will be applied if it can, and if TypeScript can't find a field with that name, it reverts to the old signature that supports a typecast.

Read more here: https://www.typescriptlang.org/docs/handbook/release-notes/typescript-2-1.html

[jtlapp]

Maybe we also need this for the new static Model.relatedQuery() in the 1.0 branch?

[koskimas]

Super cool type sorcery!

[koskimas]

@jtlapp the static relatedQuery's return type is irrelevant since it can only be used as a subquery.

[koskimas]

@mceachen People seem to use Partial<Person> and Partial<Person>[] -style relation types to make insertGraph and upsertGraph work. How will this work with those? I quess they can always use the explicit type argument?

[mceachen]

@mceachen People seem to use Partial and Partial[] -style relation types to make insertGraph and upsertGraph work. How will this work with those?

This diff shouldn't change any of the *Graph methods.

I don't believe I can do anything additional if I can't mandate that people should specify relation fields in their models.

The thing is, at least with TypeScript, model classes are pretty useless if you don't add the relation fields to them--you'd have to fight TypeScript every time you'd want to reference a relation.

Perhaps I'm being too flexible? @koskimas, what do you think, can we make this assertion? I think if we did, it would unlock additional keyof magicks.

[koskimas]

This diff shouldn't change any of the *Graph methods.

Yes, I realise that. What I meant was how will the new $relatedQuery work when this[K extends keyof this] gets expanded into Partial<SomeModel> instead of SomeModel? Will the following example work in in case somethings is of type Partial<SomeModel>[].

const result: SomeModel[] = model.$relatedQuery('somethings');

it's enough if the following still works, since that's the way it has to be done currently:

const result: SomeModel[] = model.$relatedQuery<SomeModel[]>('somethings');

[jtlapp]

Prior code that uses a cast will still work.

@mceachen, would you mind giving me an example of such a cast? The following:

    const actors = <Person[]>(await movie.$relatedQuery('actors'));

Gives me this error:

Type 'Model[]' cannot be converted to type 'Person[]'.
  Type 'Model' is not comparable to type 'Person'.
    Property 'id' is missing in type 'Model'.

I guess this works, but it offers no type-safety:

    const actors = <Person[]><any>(await movie.$relatedQuery('actors'));

[mceachen]

@jtlapp

    const actors = await movie.$relatedQuery<Person>('actors');

Examples are in examples.ts (in the diff in this PR)

[jtlapp]

Okay in that case we just have a miscommunication. I call that type parameterization and did point out that possible solution.

[jtlapp]

@mceachen, I'm still getting less-than-perfect behavior from $relatedQuery(). The following:

return person.$relatedQuery('movies', trx).insert({name: 'The Last Jedi'});

Gives me this error:

Argument of type '{ name: string; }' is not assignable to parameter of type 'Partial<Model> | undefined'.
  Object literal may only specify known properties, and 'name' does not exist in type 'Partial<Model> | undefined'.

The error goes away when I do either of the following, where MovieModel extends Model:

return person.$relatedQuery('movies', trx).insert(<MovieModel>{name: 'The Last Jedi'});

return person.$relatedQuery<MovieModel>('movies', trx).insert({name: 'The Last Jedi'});

Am I misunderstanding something here?

[jtlapp]

I guess there's no way for $relatedQuery() to return a MovieModel unless I tell it somewhere what type of object is returned, and that information isn't statically available to Typescript because the eager relation is only defined at runtime.

[jtlapp]

It seems that this new typing mainly helps when inserting Model instances?

I think I prefer the original typing of $relatedQuery(), because it returns a Model, which more clearly suggests that I need to get it working by somehow specifying the model type.

[jtlapp]

I backed out this $relatedQuery() change and am getting the same error on insertion. The error tells me that something is wrong with the argument to insert(), but there isn't. The problem doesn't occur on $query().insert(), just $relatedQuery().insert().

The solution is to call $relatedQuery<MyModel>().insert(), inclining me to think that the typings ought to produce errors that more clear direct the user to doing this.

This is just a case of getting a misleading error. Nothing is wrong with the parameter, despite the error saying so.

[jtlapp]

I think I prefer the original typing of $relatedQuery()

My apologies for that. This new problem also existed prior to the mod. The mod's looking good.