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

Typescript type definition for `.asCallback()` #2963

Merged
merged 1 commit into from Jan 16, 2019

Conversation

Projects
None yet
2 participants
@HurSungYun
Copy link
Contributor

HurSungYun commented Dec 20, 2018

In Knex version 0.16.2,

we are using Knex.js with Typescript + callback style code.

As I upgrade Knex version 0.15.0 -> 0.16.2, now it cannot be compiled typescript files anymore because of no type definition for .asCallback().

Therefore, I added .asCallback() type definition in ChainableInterface

@HurSungYun HurSungYun changed the title typescript type support for `.asCallback()` Typescript type definition for `.asCallback()` Dec 20, 2018

@HurSungYun

This comment has been minimized.

Copy link
Contributor Author

HurSungYun commented Jan 16, 2019

@kibertoad I'm really sorry to mention directly, but I can't upgrade to latest Knex version now due to this issue. Could you please review this PR? Thanks in advance for your help.

@elhigu

This comment has been minimized.

Copy link
Collaborator

elhigu commented Jan 16, 2019

So was this removed from typings between 0.15 and 0.16 or how it has gone missing?

@HurSungYun

This comment has been minimized.

Copy link
Contributor Author

HurSungYun commented Jan 16, 2019

@elhigu I believe it's missed.

Type support has been imported in this repo after this PR. #2845

However, .asCallback() type definition is not exists in that PR, and later commits.

@elhigu

This comment has been minimized.

Copy link
Collaborator

elhigu commented Jan 16, 2019

I suppose this is fine.

Funnily enough that ChainableInterface extends Bluebird<any> and bluebird itself has also method called .asCallback. So is bluebird typings also missing that?

@elhigu elhigu merged commit d857872 into tgriesser:master Jan 16, 2019

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 85.008%
Details
@HurSungYun

This comment has been minimized.

Copy link
Contributor Author

HurSungYun commented Jan 16, 2019

@elhigu Knex.js manages type definition in its own repo, but Bluebird manages their type definition in DefinitelyTyped repo. https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/bluebird/index.d.ts

So, users who use Knex.js in Typescript but haven't downloaded DefinitelyTyped type definition of Bluebird might have the same issue with me.

@HurSungYun

This comment has been minimized.

Copy link
Contributor Author

HurSungYun commented Jan 16, 2019

@elhigu I have found what is wrong.

In Knex@0.16.2, there's no "@types/bluebird" dependencies.

In Knex@0.16.3, there is type dependency for "@types/bluebird".

I tried this in Knex@0.16.2, so it was a problem. and tried to compile in 0.16.3 it has no problem.

So this PR has no meaning :(

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.