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

Sequelize Warnings with ExpenseItem and PayoutMethod #5099

Closed
znarf opened this issue Jan 7, 2022 · 4 comments · Fixed by opencollective/opencollective-api#7558
Closed
Assignees
Labels
api Issues that require some work on the API (https://github.com/opencollective/opencollective-api) bug

Comments

@znarf
Copy link
Member

znarf commented Jan 7, 2022

(sequelize) Warning: Model "ExpenseItem" is declaring public class fields for attribute(s): "id", "amount", "url", "description", "createdAt", "updatedAt", "deletedAt", "incurredAt", "ExpenseId", "CreatedByUserId".
These class fields are shadowing Sequelize's attribute getters & setters.
See https://sequelize.org/main/manual/model-basics.html#caveat-with-public-class-fields
(sequelize) Warning: Model "PayoutMethod" is declaring public class fields for attribute(s): "id", "type", "data", "createdAt", "updatedAt", "deletedAt", "name", "isSaved", "CollectiveId", "CreatedByUserId".
These class fields are shadowing Sequelize's attribute getters & setters.
See https://sequelize.org/main/manual/model-basics.html#caveat-with-public-class-fields

https://sequelize.org/main/manual/model-basics.html#caveat-with-public-class-fields

Seems this is affecting the classes built with typescript.

@znarf znarf added the bug label Jan 7, 2022
@Betree
Copy link
Member

Betree commented Jan 7, 2022

I think there's no risk on our side since the restoreSequelizeAttributesOnClass helper is supposed to restore Sequelize's getters and setters. But there might be a better way to do that nowadays, it's a good time to dig again.

@Betree Betree added the api Issues that require some work on the API (https://github.com/opencollective/opencollective-api) label Jan 7, 2022
@Betree Betree self-assigned this May 19, 2022
@Betree
Copy link
Member

Betree commented May 19, 2022

We can apparently resolve this with the declare typescript keyword, which lets you declare fields on models without overwriting them when they exist. See https://sequelize.org/docs/v6/core-concepts/model-basics/#caveat-with-public-class-fields

@ephys
Copy link

ephys commented May 19, 2022

Note that we've since learned that babel ignores declare if the class field is decorated (sequelize/sequelize#14300). restoreSequelizeAttributesOnClass is the right solution if declare does not work and we can add a flag to mute the warning if it's annoying

@Betree
Copy link
Member

Betree commented May 20, 2022

@ephys Interesting! We don't use decorators right now, so this should work for us. I started a PR to experiment with this in opencollective/opencollective-api#7558; overall things are looking good! I still have an issue in the ExpenseItem model, where somehow the model attributes still appear as undefined even though we use declare. I'll dig some more on that.

Great job on the docs BTW, when we first introduced Typescript I remember that Sequelize's docs were not super complete and I was super happy this time with all the explanations and workarounds proposed in https://sequelize.org/docs/v6/other-topics/typescript/.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Issues that require some work on the API (https://github.com/opencollective/opencollective-api) bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants