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
[types] Make Knex's default generic parameter types be assignable to knex's (TypeScript 4.6) #5021
Conversation
Can you add a tsd test for this? |
When looking into tsd/dtslint tests, I ended up hitting a dead end since some of the higher-order types like Instead, the newest version of this PR changes Furthermore I added some regression tests that fail with I understand if you don't want to take this PR since it makes the types looser. |
…knex's The default generic parameters of `Knex` didn't match the generic parameter types of `knex`. This becomes relevant with TypeScript 4.6 for `k: Knex = knex()` to work. We can't change `knex`'s declaration from `TResult = unknown[]` to `TResult = Record<string, any>[]` because other types like `DeferredKeySelection.ReplaceBase` specifically look for `unknown`. Instead, this commit changes `Knex` to use `TResult = any[]`. This works with TypeScript 4.6. Added both tsd and dtslint tests (tsd doesn't support TS 4.6 yet). Also tested this change in a larger codebase and confirmed that nearly all TS 4.6 errors were addressed.
@ide Do you think it's worth it to preserve unknown in higher-order types? Wonder if it brings value over Record. |
I don't have a strong opinion on that since I don't directly use Knex's generic parameters (my data access layer above Knex is strictly typed, and the interface between this layer and Knex is covered by tests). My original motivation was to make |
Revisiting this since TypeScript 4.6.2 has been released. I understand if you are on the fence and don't want to commit to taking this PR as the maintainer of the repo. |
@OlivierCavadenti thoughts? |
Thanks for the PR, I was having the same problem and started a fork too before seeing this one. I've introduced two types I don't know all API functions, but if Besides, TypeScript doc encourages the use of Thoughts? |
Released in 1.0.5 |
The default generic parameters of
Knex
didn't match the generic parameter types ofknex
. This becomes relevant with TypeScript 4.6 fork: Knex = knex()
to work.We can't change
knex
's declaration fromTResult = unknown[]
toTResult = Record<string, any>[]
because other types likeDeferredKeySelection.ReplaceBase
specifically look forunknown
. Instead, this commit changesKnex
to useTResult = any[]
.This works with TypeScript 4.6. Added both tsd and dtslint tests (tsd doesn't support TS 4.6 yet). Also tested this change in a larger codebase and confirmed that nearly all TS 4.6 errors were addressed.
Follow-up to #4933.