Incorrect typing of `idColumn`
According to documentation, idColumn should be set to null in the event a table does not have an id column: https://vincit.github.io/objection.js/api/model/static-properties.html#static-idcolumn
But due to the typings, null is not a possible valid value for idColumn - only string | string[]:
https://github.com/Vincit/objection.js/blob/d1e194a3ba58019a90579464459473e26668a4ab/typings/objection/index.d.ts#L1541
This causes an issue in my code on version 3.1.4. Doing either:
static idColumn = null;
or
static get idColumn() {
return null;
}
Results in a compiler error:
Types of property 'idColumn' are incompatible.
Type 'null' is not assignable to type 'string | string[]```
Only by casting like `static idColumn = null as unknown as string;` can I work around the issue. Not setting idColumn causes runtime errors.
Thanks for pointing this out @jackbonaguro. I'm happy to accept a PR that fixes this.
maybe it's better not to null, but never?
static get idColumn(): never {
throw new Error(`undefined idColumn`);
}
@lehni
after all, if there is no idColumn, then there should be no access to it at all and this should generate an error at runtime
related issue https://github.com/Vincit/objection.js/issues/2745
by the way, failed behavior:
https://github.com/Vincit/objection.js/blob/main/lib/queryBuilder/operations/InsertOperation.js#L42
if (!isSqlite(builder.knex()) && !isMySql(builder.knex()) && !builder.has(/returning/)) {
// If the user hasn't specified a `returning` clause, we make sure
// that at least the identifier is returned.
knexBuilder = knexBuilder.returning(builder.modelClass().getIdColumn());
}
if you don't specify returning in the query, then idColumn will be substituted ( => knexBuilder.returning(null);), but if it is null, then an error will occur at the query execution level...
it is better if it is at the query compilation level.
so:
https://github.com/Vincit/objection.js/pull/2746/files