Skip to content

v2.0.0: compiler error when using stricter typing Model attributes signature #882

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

Closed
spinlud opened this issue Jan 18, 2021 · 8 comments
Closed
Labels

Comments

@spinlud
Copy link

spinlud commented Jan 18, 2021

It seems there are still problems when defining models with stricter typing syntax on attributes, as already reported here #844.
The following code does not compile:

import { Optional } from 'sequelize';
import { Sequelize, Model } from "sequelize-typescript";

interface PersonAttributes {
    id: number;
    name: string;
    country?: string;
}

interface PersonCreationAttributes extends Optional<PersonAttributes, 'country'> {}

class Person extends Model<PersonAttributes, PersonCreationAttributes> implements PersonAttributes {
    id!: number;
    name!: string;
    country?: string;
}


(async () => {
    const sequelize =  new Sequelize({
        dialect: 'mysql',
        host: 'localhost',
        port: 3306,
        database: 'testdb',
        username: 'admin',
        password: 'admin',
        logQueryParameters: true,
        logging: true,
    });

    await sequelize.authenticate();

    await sequelize.addModels([Person]); // <-- compiler complains

    await sequelize.close();
})();

image

See also:

@theoludwig theoludwig added the bug label Jan 21, 2021
@RobinBuschmann
Copy link
Member

RobinBuschmann commented Jan 24, 2021

Hey @spinlud, thanks for reporting.

This doesn't seem to be a sequelize-typescript (only) issue. Similar code also throws a compiler error with pure sequelize:

import {Model} from 'sequelize';

class Test extends Model<{test: string}> {}
class Test2 extends Model {}

Test2.findAll({
  include: [Test], // fails here
});
TS2322: Type 'typeof Test' is not assignable to type 'Includeable'.   
  Type 'typeof Test' is not assignable to type 'typeof Model'.     
    Types of construct signatures are incompatible.       
      Type 'new (values?: { test: string; } | undefined, options?: BuildOptions | undefined) => Test' is not assignable to type 'new <TModelAttributes extends {} = any, TCreationAttributes extends {} = TModelAttributes>(values?: TCreationAttributes | undefined, options?: BuildOptions | undefined) => Model<TModelAttributes, TCreationAttributes>'.        
      Types of parameters 'values' and 'values' are incompatible.           
        Type 'TCreationAttributes | undefined' is not assignable to type '{ test: string; } | undefined'.             
          Type 'TCreationAttributes' is not assignable to type '{ test: string; } | undefined'.               
            Type '{}' is not assignable to type '{ test: string; }'.                 
              Type 'TCreationAttributes' is not assignable to type '{ test: string; }'.                   
                Property 'test' is missing in type '{}' but required in type '{ test: string; }'.

Can anyone confirm?

The source of the problem seems to be the default value of generic TCreationAttributes in the definition of Model(sequelize).

See following example:

class Model<TModelAttributes = any, TCreationAttributes = TModelAttributes> {
    constructor(values?: TCreationAttributes) {}
}

class Test extends Model<{test: string}> {}

const addModel = (model: typeof Model) => {};

addModel(Test);

(see in typescript playground)
If we change TCreationAttributes = TModelAttributes to TCreationAttributes = any the error disappears. Which is strange, since TCreationAttributes should be inferred to any anyway.

I'm considering to just use type ModelCtor<M extends Model = any> = (new (...args: any[]) => M) instead of export declare type ModelCtor<M extends Model = any> = (new () => M) & ModelType;(so omitting ModelType, which is an alias for typeof Model) in sequelize-typescriot. It fixes the problem and should give "enough" type safety, I think. But the IncludeOptions und the Includable type of sequelize need to be fixed as well: https://github.com/sequelize/sequelize/blob/main/types/lib/model.d.ts#L394

@RoelVB
Copy link

RoelVB commented Jan 25, 2021

@RobinBuschmann I can confirm I'm seeing the same error with just sequelize.
Your playground example is very interesting 🤔

Found a related issue on the sequelize side:
sequelize/sequelize#12842

@AlexanderProd
Copy link

It seems like, for now the only workaround is to use //@ts-ignore where this happens, right?

@RoelVB
Copy link

RoelVB commented Feb 15, 2021

@AlexanderProd Yes, I've been ignoring the errors, didn't run into any problems yet.
But... I guess PR #900 would fix this.

-edit-
Looks like 2.1.0 was released a couple of hours ago, I will check it out tomorrow.

@AlexanderProd
Copy link

AlexanderProd commented Feb 15, 2021

@RoelVB
The problem still happens in 2.1.0 for me.

@RoelVB
Copy link

RoelVB commented Feb 16, 2021

@AlexanderProd I just tested [email protected] with [email protected] and the problem described in this issue disappeared.

I only noticed the typings issue is still there with includes. This is waiting for sequelize/sequelize#13010

@KapitanOczywisty
Copy link
Contributor

Issue should be fixed in sequelize 6.6.1, please verify. @divlo

@spinlud
Copy link
Author

spinlud commented Mar 27, 2021

Apologies for the delay.
Yes I can confirm that with the new versions the problem is solved.

I've also updated sequelize-typescript-generator to leverage the strict mode as well.

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

No branches or pull requests

6 participants