-
Notifications
You must be signed in to change notification settings - Fork 641
Fix idColumn + jsonAttributes type #2746
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
@@ -1442,11 +1442,11 @@ declare namespace Objection { | |||
QueryBuilder: typeof QueryBuilder; | |||
|
|||
tableName: string; | |||
idColumn: string | string[]; | |||
idColumn: string | string[] | readonly string[] | never; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what does never
achieve here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the table model may not have any column id at all (not an empty array - but simply a method, for example, always throw out an exception)
static get idColumn(): never {
throw new Error(`undefined id for ..some.. table`);
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What would be the purpose of an idColumn gett that throws an exception?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
goal - if the table does not have id - it cannot be used in runtime.
this should generate an exception, and not allow further compilation of SQL code.
and using never as a return value for the idColumn function will allow wrapping the code at the static compilation level, any use will generate a compile time error.
details here #2693 (comment)
the very reference to idColumn, when it does not exist, is a code logic error!
and already now if you allow null then there will be an error during the assembly of certain queries!
this will introduce regression into the code base!
fixes #2745