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 Definitions: fix Knex.QueryBuilder.extend signature #3526

Closed
iki opened this issue Nov 8, 2019 · 2 comments · Fixed by #3528
Closed

TypeScript Definitions: fix Knex.QueryBuilder.extend signature #3526

iki opened this issue Nov 8, 2019 · 2 comments · Fixed by #3528
Labels
bug

Comments

@iki
Copy link
Contributor

@iki iki commented Nov 8, 2019

Environment

Knex version: 0.19.5

Bug

  1. Explain what kind of behaviour you are getting and how you think it should do

Function passed to Knex.QueryBuilder.extend uses this as QueryBuilder . However it is typed as Knex.

  1. Error message

TypeScript complains about QueryBuilder methods like toSQL or Promise methods like then when used on this in the extending function.

Property 'then' does not exist on type Knex<any, any[]>

  1. Reduced test code

See #1355 (comment).

It uses this: Knex.QueryBuilder in function signature to fix the problem and types the fuction as any in the end to avoid TypeScript complaining about unassignable function types due to Knex and QueryBuilder type incompatibility.

  1. Solution

The Knex.QueryBuilder.extend signature should use QueryBuilder for this:

static extend(
    methodName: string,
    fn: <TRecord extends {} = any, TResult = unknown[]>(
      this: QueryBuilder<TRecord, TResult>,
      ...args: any[]
    ) => QueryBuilder<TRecord, TResult>
  ): void;

instead of Knex:

static extend(
    methodName: string,
    fn: <TRecord extends {} = any, TResult = unknown[]>(
      this: Knex<TRecord, TResult>,
      ...args: any[]
    ) => QueryBuilder<TRecord, TResult>
  ): void;
@lorefnon lorefnon added the bug label Nov 8, 2019
@lorefnon

This comment has been minimized.

Copy link
Collaborator

@lorefnon lorefnon commented Nov 8, 2019

@iki Since you have already figured out the solution, would you consider creating a PR for this ?

iki added a commit to iki/knex that referenced this issue Nov 8, 2019
iki added a commit to iki/knex that referenced this issue Nov 28, 2019
iki added a commit to iki/knex that referenced this issue Dec 7, 2019
kibertoad added a commit that referenced this issue Dec 7, 2019
@kibertoad

This comment has been minimized.

Copy link
Collaborator

@kibertoad kibertoad commented Dec 7, 2019

Released in 0.20.4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.