Skip to content
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

Fix backward-incompatibility in type definitions #3189

Merged
merged 1 commit into from May 12, 2019
Merged

Fix backward-incompatibility in type definitions #3189

merged 1 commit into from May 12, 2019

Conversation

@lorefnon
Copy link
Collaborator

@lorefnon lorefnon commented May 12, 2019

Changes in #3168 introduces backward incompatibility causing some other type definitions on DefinitivelyTyped to break when used with latest Knex.

This PR (tested with all the external type definitions in DefinitivelyTyped having knex as dependency) addresses them by:

  • Adding defaults for parameters of all exposed types which are now generic
  • Allowing query builders to be used as select arguments (to accomodate as usage).
@kibertoad
Copy link
Collaborator

@kibertoad kibertoad commented May 12, 2019

Shoud dtslint tests be updated?

@lorefnon
Copy link
Collaborator Author

@lorefnon lorefnon commented May 12, 2019

@kibertoad I have added specs for as usage, which was affected by this change. I am not sure if something needs to be added for the default type args.

Do you have any suggestions here ?

@kibertoad
Copy link
Collaborator

@kibertoad kibertoad commented May 12, 2019

That should suffice! CI is failing now, though.

@kibertoad
Copy link
Collaborator

@kibertoad kibertoad commented May 12, 2019

@lorefnon

ERROR: 188:3   expect  Expected type to be:
  Pick<Partial<any>, string>[]
got:
  any
ERROR: 188:30  expect  TypeScript@next compile error: 
Argument of type 'QueryBuilder<any, DeferredKeySelection<any, "bar">[]>' is not assignable to parameter of type 'ColumnDescriptor<any, unknown[]>[]'.
  Type 'QueryBuilder<any, DeferredKeySelection<any, "bar">[]>' is missing the following properties from type 'ColumnDescriptor<any, unknown[]>[]': length, pop, push, concat, and 21 more.

Looks like problem is with new test.

- Allow query builders to be used as select arguments.
- Add default values for generic types
@lorefnon
Copy link
Collaborator Author

@lorefnon lorefnon commented May 12, 2019

Yes, typescript master has some changes around type inference which caused it to fail with typescript@next. It is fixed now.

@kibertoad kibertoad merged commit 1f4c883 into knex:master May 12, 2019
1 of 2 checks passed
@kibertoad
Copy link
Collaborator

@kibertoad kibertoad commented May 12, 2019

Thanks!

@kibertoad
Copy link
Collaborator

@kibertoad kibertoad commented May 12, 2019

@lorefnon 0.16.7 is out. Would appreciate if you could confirm if it resolves broken backwards compatibility!

@lorefnon
Copy link
Collaborator Author

@lorefnon lorefnon commented May 12, 2019

Thanks for releasing. I have tried this in a sample application and verified that with both typescript@latest and typescript@next the errors are resolved now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants