Nullable expressions: undefined -> null #2
Conversation
Fixes improper type resolution of HasDefault. Still need to update tests.
refactor types in result-set to make capturing role clearer.
…uery types harder to read in usages.
…ue or not. Use function overloading to handle issue with incorrect nullability with no type arg.
| @@ -7,6 +7,8 @@ export type PickByValue<T, ValueType> = Pick< | |||
| }[keyof T] | |||
| >; | |||
|
|
|||
| export type Expand<T> = T extends infer O ? { [K in keyof O]: O[K] } : never; | |||
There was a problem hiding this comment.
Maybe a comment for what this does?
| "js" | ||
| ], | ||
| "coveragePathIgnorePatterns": [ | ||
| "__tests__", |
There was a problem hiding this comment.
The old code used __tests__ and __checks__ folders. Can we keep that structure or is that hard?
(If we're starting out with the hope that this fork goes away eventually, it might be nice to avoid unnecessary changes.)
There was a problem hiding this comment.
I moved this for less ignore configuration in these files, but if you'd prefer I can switch it back.
There was a problem hiding this comment.
Yeah, I think I'd prefer to leave it unchanged in this case, to stay close to the upstream code.
(I don't think that's always the right answer, e.g. changing the type testing library like a change that's worth it.)
src/insert.ts
Outdated
| @@ -327,7 +327,7 @@ export class InsertQuery< | |||
| > | |||
| ? IsNotNull extends true | |||
| ? DataType | Expression<DataType, IsNotNull, any> | Query<any> | |||
| : DataType | undefined | Expression<DataType, IsNotNull, any> | Query<any> | |||
| : DataType | null | Expression<DataType, IsNotNull, any> | Query<any> | |||
There was a problem hiding this comment.
Maybe worth defining a type DbNull = null somewhere and using that everywhere instead of the literal null?
(Right now, when I see null, I'm not sure if it represents a DB null value or if it's just one of the normal TS uses of null.)
src/sql-functions.ts
Outdated
| import { DefaultExpression, Expression } from './expression'; | ||
|
|
||
| import { Query } from './query'; | ||
| import { Table } from './TableType'; | ||
|
|
||
| export interface Tokenable { | ||
| toTokens(): Token[]; | ||
| } |
There was a problem hiding this comment.
Weird indentation...
I thought this project had a prettier?
nulldata type instead ofundefined.jest-runner-tsd. These tests now can check on the actual result set data type, rather than having to use theGetDataTypeclass to extract nullability.GetDataTypehelper is now only used inFromItemQuery, rather than for testing only.toSqlhelper to acceptTokenabletypes.