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 typeof Model errors throughout by using typeof Model generics #868

Closed

Conversation

intellix
Copy link
Contributor

@intellix intellix commented Dec 13, 2020

Hopefully addresses #835

This is essentially me just applying it in the places that worked and then re-applying it throughout until typescript errors disappeared and it compiled/built.

It builds and all tests pass. When I include it in my own project I go from 457 errors to 107 and I don't see my original problems as in issue #835.

  493 passing (27s)
  6 pending


=============================== Coverage summary ===============================
Statements   : 97.95% ( 1098/1121 )
Branches     : 89.46% ( 280/313 )
Functions    : 97.27% ( 249/256 )
Lines        : 97.99% ( 1075/1097 )
================================================================================

I'm still getting a problem with Includeable from Sequelize as it expects typeof Model. The new ModelType in this PR would fix it, but how do you patch existing types?
For example:

PaymentCard.findOne({
  transaction,
  include: [BillingAddress], <-------------
  where: { active: true },
}),

Gives:

No overload matches this call.
  Overload 1 of 2, '(this: ModelStatic<PaymentCard>, options: NonNullFindOptions<PaymentCardAttrs>): Promise<PaymentCard>', gave the following error.
    Type 'typeof BillingAddress' is not assignable to type 'Includeable'.
      Type 'typeof BillingAddress' is not assignable to type 'typeof Model'.
        Types of construct signatures are incompatible.
          Type 'new (values?: BillingAddressCreateAttrs, options?: BuildOptions) => BillingAddress' is not assignable to type 'new <TModelAttributes extends {} = any, TCreationAttributes extends {} = TModelAttributes>(values?: TCreationAttributes, options?: BuildOptions) => Model<TModelAttributes, TCreationAttributes>'.
            Types of parameters 'values' and 'values' are incompatible.
              Type 'TCreationAttributes' is not assignable to type 'BillingAddressCreateAttrs'.
                Type '{}' is missing the following properties from type 'BillingAddressCreateAttrs': paymentCardId, name, lastName, address1, and 6 more.
  Overload 2 of 2, '(this: ModelStatic<PaymentCard>, options?: FindOptions<PaymentCardAttrs>): Promise<PaymentCard>', gave the following error.
    Type 'typeof BillingAddress' is not assignable to type 'Includeable'.
      Type 'typeof BillingAddress' is not assignable to type 'typeof Model'.ts(2769)

Feel free to take over, base your own work on this or throw it away entirely. I'm not attached to it :)

@intellix intellix marked this pull request as ready for review December 14, 2020 00:14
@hsluoyz
Copy link

hsluoyz commented Jan 10, 2021

@intellix

I forked your PR: https://github.com/node-casbin/sequelize-ts and ran the test with the following errors:

"C:\Program Files\nodejs\node.exe" "C:\Program Files\nodejs\node_modules\npm\bin\npm-cli.js" test --scripts-prepend-node-path=auto

> [email protected] test F:\github_repos\sequelize-ts
> mocha test/**/*.spec.ts


TSError: ⨯ Unable to compile TypeScript:
src/associations/belongs-to-many/belongs-to-many-association.ts(32,54): error TS2345: Argument of type 'typeof Model' is not assignable to parameter of type 'Model
Type<{}, {}>'.
  Cannot assign an abstract constructor type to a non-abstract constructor type.
src/associations/belongs-to-many/belongs-to-many-association.ts(33,62): error TS2345: Argument of type 'typeof Model' is not assignable to parameter of type 'Model
Type<{}, {}>'.

    at createTSError (F:\github_repos\sequelize-ts\node_modules\ts-node\src\index.ts:261:12)
    at getOutput (F:\github_repos\sequelize-ts\node_modules\ts-node\src\index.ts:367:40)
    at Object.compile (F:\github_repos\sequelize-ts\node_modules\ts-node\src\index.ts:558:11)
    at Module.m._compile (F:\github_repos\sequelize-ts\node_modules\ts-node\src\index.ts:439:43)
    at Module._extensions..js (internal/modules/cjs/loader.js:1092:10)
    at Object.require.extensions.<computed> [as .ts] (F:\github_repos\sequelize-ts\node_modules\ts-node\src\index.ts:442:12)
    at Module.load (internal/modules/cjs/loader.js:928:32)
    at Function.Module._load (internal/modules/cjs/loader.js:769:14)
    at Module.require (internal/modules/cjs/loader.js:952:19)
    at require (internal/modules/cjs/helpers.js:88:18)
    at Object.<anonymous> (F:\github_repos\sequelize-ts\src\associations\belongs-to-many\belongs-to-many.ts:2:1)
    at Module._compile (internal/modules/cjs/loader.js:1063:30)
    at Module.m._compile (F:\github_repos\sequelize-ts\node_modules\ts-node\src\index.ts:439:23)
    at Module._extensions..js (internal/modules/cjs/loader.js:1092:10)
    at Object.require.extensions.<computed> [as .ts] (F:\github_repos\sequelize-ts\node_modules\ts-node\src\index.ts:442:12)
    at Module.load (internal/modules/cjs/loader.js:928:32)
    at Function.Module._load (internal/modules/cjs/loader.js:769:14)
    at Module.require (internal/modules/cjs/loader.js:952:19)
    at require (internal/modules/cjs/helpers.js:88:18)
    at Object.<anonymous> (F:\github_repos\sequelize-ts\test\specs\annotations\belongs-to-many.spec.ts:6:1)
    at Module._compile (internal/modules/cjs/loader.js:1063:30)
    at Module.m._compile (F:\github_repos\sequelize-ts\node_modules\ts-node\src\index.ts:439:23)
    at Module._extensions..js (internal/modules/cjs/loader.js:1092:10)
    at Object.require.extensions.<computed> [as .ts] (F:\github_repos\sequelize-ts\node_modules\ts-node\src\index.ts:442:12)
    at Module.load (internal/modules/cjs/loader.js:928:32)
    at Function.Module._load (internal/modules/cjs/loader.js:769:14)
    at Module.require (internal/modules/cjs/loader.js:952:19)
    at require (internal/modules/cjs/helpers.js:88:18)
    at Object.exports.requireOrImport (F:\github_repos\sequelize-ts\node_modules\mocha\lib\esm-utils.js:20:12)
    at Object.exports.loadFilesAsync (F:\github_repos\sequelize-ts\node_modules\mocha\lib\esm-utils.js:33:34)
    at Mocha.loadFilesAsync (F:\github_repos\sequelize-ts\node_modules\mocha\lib\mocha.js:421:19)
    at singleRun (F:\github_repos\sequelize-ts\node_modules\mocha\lib\cli\run-helpers.js:156:15)
    at exports.runMocha (F:\github_repos\sequelize-ts\node_modules\mocha\lib\cli\run-helpers.js:225:10)
    at Object.exports.handler (F:\github_repos\sequelize-ts\node_modules\mocha\lib\cli\run.js:366:11)
    at F:\github_repos\sequelize-ts\node_modules\mocha\node_modules\yargs\lib\command.js:241:49
npm ERR! Test failed.  See above for more details.

Process finished with exit code 1

image

Copy link
Contributor

@theoludwig theoludwig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Node.js CI is failing, it should succeeds.

@theoludwig theoludwig added the bug label Jan 21, 2021
@intellix
Copy link
Contributor Author

I'm not personally gonna spend anymore time on this. With the help from someone in Gitter I managed to produce this but then there were many more issues with it.

At this point it's at the "WTF Typescript" stage that I have no clue what is going on. Someone else with more knowledge is going to have to take over

@KapitanOczywisty
Copy link
Contributor

@intellix Build is failing on formatting, run npm run lint --fix to reformat. After that there are 2 more errors on npm run build:

src/associations/belongs-to-many/belongs-to-many-association.ts:32:54 - error TS2345: Argument of type 'typeof Model | undefined' is not assignable to parameter of type 'ModelType<{}, {}> | undefined'.
  Type 'typeof Model' is not assignable to type 'ModelType<{}, {}>'.
    Cannot assign an abstract constructor type to a non-abstract constructor type.

32     options.foreignKey = getForeignKeyOptions(model, throughModel, this.options.foreignKey);
                                                        ~~~~~~~~~~~~

src/associations/belongs-to-many/belongs-to-many-association.ts:33:62 - error TS2345: Argument of type 'typeof Model | undefined' is not assignable to parameter of type 'ModelType<{}, {}> | undefined'.
  Type 'typeof Model' is not assignable to type 'ModelType<{}, {}>'.

33     options.otherKey = getForeignKeyOptions(associatedClass, throughModel, this.options.otherKey);
                                                                ~~~~~~~~~~~~

With throughModel as any hack. build is successfull and seems to be working fine, so it's pretty close.

@theoludwig
Copy link
Contributor

theoludwig commented Feb 6, 2021

@KapitanOczywisty Could open a new PR, to fix this bug with the CI that succeeds, please ?
So we can close this PR, since @intellix don't have any more time to work on this.
Thanks for your contribution!

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

Successfully merging this pull request may close these issues.

4 participants