feat(core): simplify entity definition and rework typings of FilterQuery

[B4nan]

Now it is no longer needed to merge entities with IEntity interface, that was polluting entity's interface with internal methods.
New interfaces IdentifiedEntity<T>, UuidEntity<T> and MongoEntity<T> are introduced, that should be implemented by entities. They
are not adding any new properties or methods, keeping the entity's interface clean.

This also introduces new strictly typed FilterQuery<T> implementation based on information provided by those new interfaces (type of primary key
in particular). It is not used in query builder, as there string keys needs to be supported for aliasing to work ({ 'a.books': 1 }).

BREAKING CHANGES:
IEntity interface no longer has public methods like toJSON(), toObject() or init(). One can use wrap() method provided by ORM that
will enhance property type when needed with those methods (await wrap(book.author).init()). To keep all methods available on the
entity, you can still use interface merging with WrappedEntity<T, PK> that both extends IEntity<T, PK> and defines all those methods.
FilterQuery now does not allow using smart query operators. You can either cast your condition as any or use object syntax instead
(instead of { 'age:gte': 18 } use { age: { $gte: 18 } }).

Closes #124, #171

[B4nan]

Example of entity definition in SQL drivers (integer AI primary key):

@Entity()
export class FooBar implements IdEntity<FooBar> {

  @PrimaryKey()
  id: number;

  @Property()
  name: string;

}

For mongo it is a bit more complicated as now both _id and id will need to be defined:

@Entity()
export class FooBar implements MongoEntity<FooBar> {

  @PrimaryKey()
  _id: ObjectId;

  @SerializedPrimaryKey()
  id: string;

  @Property()
  name: string;

}

[B4nan]

Not yet fully polished, but would love to hear your feedback @lookfirst, @vimmerru, @pepakriz.

Available for testing via dev tag (3.0.0-dev.1).

[lookfirst]

Damn, no wonder you've been so quiet recently. Good work!

[pepakriz]

I have just a little experience with the orm code, but it looks very impressive!

[thynson]

Wow, you made it so quickly. Excellent work!

[lookfirst]

What if I want an id: string?

[B4nan]

What if I want an id: string?

There is still IEntity interface for that, where you specify what property is PK and its type will be used. There is also PrimaryKeyType symbol that can be used to make sure the type is correctly set in FilterQuery, but it should not be needed (at least with TS 3.6 it looks like that, I added that symbol when developing it on TS 3.5 but it might work there as well).

@Entity()
export class FooBar implements IEntity<Test2, 'id'> {

  @PrimaryKey()
  id: string;

}

Btw just found out there are still some issues with FilterQuery, it allows to put anything into property on the top level (nested conditions are correctly checked though):

const ok: FilterQuery<Author2> = { books: { author: { born: new Date() } } };
const fail: FilterQuery<Author2> = { books: { undef: { born: new Date() } } };
const shouldFail: FilterQuery<Author2> = { undef: { author: { born: new Date() } } };

[lookfirst]

Yea, don't worry about supporting older versions of TS. Happy to stay current with that. Running 3.6.2 and may even switch to the 3.7 beta soon to get all the awesome ? improvements.

Re: IEntity, I must admit that it feels awkward to have to switch between two different interfaces and formats just because I'm using a string or number primary key.

As always, excellent work. Super appreciate.

[B4nan]

Re: IEntity, I must admit that it feels awkward to have to switch between two different interfaces and formats just because I'm using a string or number primary key.

You can use IEntity for all entities if you want. Those extra interfaces are there just to simplify things, usually one will use the same type of PKs in all entities imho.

export type IEntity<T = any, PK extends keyof T = keyof T> = { [K in PK]: T[K] };
export type WrappedEntity<T, PK extends keyof T> = IWrappedEntity<T, PK> & IEntity<T, PK>;
export type IdentifiedEntity<T extends { id: number }> = IEntity<T, 'id'>;
export type UuidEntity<T extends { uuid: string }> = IEntity<T, 'uuid'>;
export type MongoEntity<T extends { _id: IPrimaryKey; id: string }> = IEntity<T, 'id' | '_id'>;

[lookfirst]

Maybe IEntity should be AnyEntity and IdentifiedEntity should be NumericEntity

[lookfirst]

Or NumberEntity

[B4nan]

I picked IdentifiedEntity because of the requirement for id property... I think we could support both number and string there, important is to know the property name, its type can be inferred.

Not sure about AnyEntity, lets wait for other opinions, I am not against renaming it, I know the name is too generic (but - generic as the interface itself). It is used to define any entity, so maybe it could be good name.

[thynson]

Why not

type IdentifiedEntity<T extends { id: R}, R=number> = IEntity<T, 'id'>;

so user has the ability to specify the type of primary key

[B4nan]

That's pretty much what I just said in last comment :] Will make this happen, was thinking about it anyway...

I even think we could just have (other types of PK than string and number are not supported, mongo requires _id property so no need to take ObjectId into account here):

type IdentifiedEntity<T extends { id: number | string }> = IEntity<T, 'id'>;

[lookfirst]

I am not a fan of prefixing interfaces with I, it is a cheapsauce naming technique. interfaces are the things you use, not just look at.

[B4nan]

I am not a fan of prefixing interfaces with I, it is a cheapsauce naming technique. interfaces are the things you use, not just look at.

Me neither, but... naming is hard, you know :D

Let's call it AnyEntity unless someone proposes something better.

[lookfirst]

Change...

export type IEntity<T = any, PK extends keyof T = keyof T> = { [K in PK]: T[K] };
export type IdentifiedEntity<T extends { id: number }> = IEntity<T, 'id'>;

into...

export type AnyEntity<T = any, PK extends keyof T = keyof T> = { [K in PK]: T[K] };
export type IdEntity<T extends { id: number | string }> = AnyEntity<T, 'id'>;

Seems sane and now is a good time given the breaking changes...

[B4nan]

Ok done with the FilterQuery issue hopefully, also renamed IEntity and IdentifiedEntity to AnyEntity and IdEntity plus IdEntity now allows using both number and string PK.

Published as 3.0.0-dev.3.

[B4nan]

Found one more small issue with FilterQuery: using operators with properties of Date type are failing (e.g. { born: { $gte: new Date('...') } }), but that should be hopefully easy to fix.

edit: Right, it was just wrong OperatorMap definition as $gte and other comparison operators can be used with more than just a number... released 3.0.0-dev.4 with the fix.

[pepakriz]

I'm not sure about FilterQuery typing. The current behavior is unfriendly with private properties for example. I've been thinking about typing and I'd like to have cli command which generates TS types. This principle is used by prisma and it has one great advantage - you're not limited by TS. Generated types are simple as possible without TS magic which can break something every TS version. So I'd completely prefer this way.

[B4nan]

Well that was first approach I was thinking about, but I don't like the idea of generating them only via CLI, it would have to be somehow automated (which could work with enabled caching probably). Still current approach looks good to me, so I am not very keen to rewrite that... Will test it for couple more days in the wild, but so far it works good.

The current behavior is unfriendly with private properties for example.

Not sure if that is important, as private props are not really supported by the ORM (there is no mechanism that would call your setters instead of just assigning the value internally). Any other issue with current approach?

Generated types are simple as possible without TS magic

Still they would be pretty complex as well I think, although definitely less generic/more specific. I did not pay much attention to prisma yet, but from what I saw, they have more strict rules on the query type, e.g. they do not allow query by PK ({ author: { id: 1 } } instead of { author: 1 } ).

Let's see what others are thinking.

Edit: One more note - I don't wanna postpone v3 release for too long, but I am definitely open for discussion about this topic for future versions. That said, I think the current approach will be enough for v3.

[lookfirst]

Example project updated, all tests pass. No complaints. =) After midnight here, so I'll do my main project tomorrow morning.

[B4nan]

Merging this now and closing both connected issues, but feel free to continue with discussion. For bugs please open new issues.