Sitelet https://web.archive.org/web/20210823195915/https://github.com/sequelize/sequelize/issues/13302
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

Incorrect typescript typings for findAndCountAll function #13302

Open
3 of 7 tasks
vinay-naik opened this issue Jun 6, 2021 · 2 comments · May be fixed by #13303
Open
3 of 7 tasks

Incorrect typescript typings for findAndCountAll function #13302

vinay-naik opened this issue Jun 6, 2021 · 2 comments · May be fixed by #13303

Comments

@vinay-naik
Copy link
Task lists! Give feedback

@vinay-naik vinay-naik commented Jun 6, 2021 •

Issue Description

What are you doing?

I am trying to strongly type my application using Typescript and Sequelize. I have noticed the following behaviour of the findAndCountAll function.

  1. When no group by is supplied it returns Promise<{ rows: M[]; count: number }>.
  2. When group by is supplied it returns Promise<{ rows: M[]; count: { count: number }[]>>

The current type of Sequelize findAndCountAll is

  public static findAndCountAll<M extends Model>(
    this: ModelStatic<M>,
    options?: FindAndCountOptions<M['_attributes']>
  ): Promise<{ rows: M[]; count: number }>;

What do you expect to happen?

Since findAndCountAll returns two different results the type should ideally be

  public static findAndCountAll<M extends Model>(
    this: ModelStatic<M>,
    options?: FindAndCountOptions<M['_attributes']>
  ): Promise<{ rows: M[]; count: number | { count: number }[]>;

I have noticed other attributes also being returned inside the count object so just a count : { count: number } might not work.

What is actually happening?

As discussed in this issue to return the proper count I need to take a count of the returned count array. But since the type says count is a number I am unable to do so.

    const allUsersRaw = await this.User.findAndCountAll({
        attributes,
        include,
        where,
        order,
        limit,
        offset,
        group: ["Order.id"],
        subQuery: false
    });
    const count = allUsersRaw.count.length;

Following error is thrown

Property 'length' does not exist on type 'number'. ts(2339)

Environment

  • Sequelize version: 6.6.2
  • Node.js version: v16.3.0
  • Operating System: MacOs BigSur 11.4
  • If TypeScript related: TypeScript version: 4.3.2

Issue Template Checklist

How does this problem relate to dialects?

  • I think this problem happens regardless of the dialect.
  • I think this problem happens only for the following dialect(s):
  • I don't know, I was using PUT-YOUR-DIALECT-HERE, with connector library version XXX and database version XXX

Would you be willing to resolve this issue by submitting a Pull Request?

  • Yes, I have the time and I know how to start.
  • Yes, I have the time but I don't know how to start, I would need guidance.
  • No, I don't have the time, although I believe I could do it if I had the time...
  • No, I don't have the time and I wouldn't even know how to start.
@vinay-naik vinay-naik changed the title Incorrect typings For findAndCountAll Incorrect typings for *findAndCountAll* function Jun 6, 2021
@vinay-naik vinay-naik changed the title Incorrect typings for *findAndCountAll* function Incorrect typescript typings for findAndCountAll function Jun 6, 2021
@Keimeno
Copy link
Member

@Keimeno Keimeno commented Jun 6, 2021

Ideally, this should be done with function overloading.

public static findAndCountAll<M extends Model>(
  this: ModelStatic<M>,
  options?: FindAndCountOptions<M['_attributes']> & {group: undefined}
): Promise<{ rows: M[]; count: number }>;
public static findAndCountAll<M extends Model>(
  this: ModelStatic<M>,
  options?: FindAndCountOptions<M['_attributes']> & {group: GroupOption}
): Promise<{ rows: M[]; count: number[] }>;

This allows us to dynamically generate a return type based on the developer's input

@Patil2099
Copy link

@Patil2099 Patil2099 commented Jun 7, 2021

I would love to take up this issue @Keimeno.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked pull requests

Successfully merging a pull request may close this issue.

3 participants