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

Conversation

Projects
None yet
2 participants
@lorefnon
Copy link
Contributor

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

This comment has been minimized.

Copy link
Collaborator

commented May 12, 2019

Shoud dtslint tests be updated?

@lorefnon lorefnon force-pushed the lorefnon:types-backward-compat branch from 352a0e7 to 48bbede May 12, 2019

@lorefnon

This comment has been minimized.

Copy link
Contributor Author

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

This comment has been minimized.

Copy link
Collaborator

commented May 12, 2019

That should suffice! CI is failing now, though.

@kibertoad

This comment has been minimized.

Copy link
Collaborator

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.

@lorefnon lorefnon force-pushed the lorefnon:types-backward-compat branch from 48bbede to df1bdb9 May 12, 2019

Fix backward-incompatibility:
- Allow query builders to be used as select arguments.
- Add default values for generic types

@lorefnon lorefnon force-pushed the lorefnon:types-backward-compat branch from df1bdb9 to 149da70 May 12, 2019

@lorefnon

This comment has been minimized.

Copy link
Contributor Author

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 tgriesser:master May 12, 2019

1 of 2 checks passed

coverage/coveralls Coverage decreased (-0.09%) to 85.236%
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@kibertoad

This comment has been minimized.

Copy link
Collaborator

commented May 12, 2019

Thanks!

@kibertoad

This comment has been minimized.

Copy link
Collaborator

commented May 12, 2019

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

@lorefnon

This comment has been minimized.

Copy link
Contributor Author

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
You can’t perform that action at this time.