FindConditions instead of any for EntityManager.delete()

[osdiab]

Context is in #4608 . I'm not 100% sure if this is correct for using delete properly (it passes the code directly to a QueryBuilder.where() clause, which I think can take FindConditions? But I'm not totally sure if that's how QueryBuilder works.

Having something like this would save my team and probably other's from blowing up their own dbs.

[pleerock]

it won't support all FindConditions features, only some trivial ones

[Kononnable]

Ok, so what should we do about this PR? Close it or change FindConditions to some subtype of it which would contain only features that work with delete? There are also tests missing which I consider necessary with such API.

@osdiab Are you still interested in this PR?

[osdiab]

It would have been nice for past me but I'm used to it now that I'm not messing as much with the particular code - I do think it should probably go through in some form but I'm not sure what exactly to put there, as you've highlighted. If there's documentation somewhere about what delete can do that would be useful, otherwise I can delve into the delete code and grok it directly

[osdiab]

This is the query builder code that criteria eventually gets used in: https://github.com/typeorm/typeorm/blob/master/src/query-builder/DeleteQueryBuilder.ts#L144

[osdiab]

That would imply that a general, potentially correct option might be string|((qb: QueryBuilder<Entity>) => string)|Brackets|ObjectLiteral|ObjectLiteral[], though that is quite general/footgun prone

[pleerock]

On the next branch we already have this change, and I suppose because its working better in there.

[AlexMesser]

Closing for now.