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(types): add instance member declaration #13684

Merged
merged 3 commits into from Nov 22, 2021
Merged

Conversation

joe223
Copy link
Contributor

@joe223 joe223 commented Nov 19, 2021

Pull Request Checklist

Please make sure to review and check all of these items:

  • Have you added new tests to prevent regressions?
  • Does npm run test or npm run test-DIALECT pass with this change (including linting)?
  • Is a documentation update included (if this change modifies existing APIs, or introduces new ones)?
  • Did you update the typescript typings accordingly (if applicable)?
  • Does the description below contain a link to an existing issue (Closes #[issue]) or a description of the issue you are solving?
  • Did you follow the commit message conventions explained in CONTRIBUTING.md?

Description Of Change

Todos

sequelize/lib/sequelize.js

Lines 1257 to 1264 in 51bc05a

Sequelize.prototype.fn = Sequelize.fn;
Sequelize.prototype.col = Sequelize.col;
Sequelize.prototype.cast = Sequelize.cast;
Sequelize.prototype.literal = Sequelize.literal;
Sequelize.prototype.and = Sequelize.and;
Sequelize.prototype.or = Sequelize.or;
Sequelize.prototype.json = Sequelize.json;
Sequelize.prototype.where = Sequelize.where;

These methods are not only static member of Sequelize. So I made this PR and trying to keep the definition and behavior consistent.

Copy link
Member

@sdepold sdepold left a comment

Choose a reason for hiding this comment

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

Could you please add a test to the types area of the repo?
I think this would be a good place: https://github.com/sequelize/sequelize/blob/main/types/test/sequelize.ts

Prior to your change you should not have been able to do sequelize.cast(1, 'string') for example.

@sdepold sdepold self-assigned this Nov 19, 2021
@sdepold sdepold added type: typescript For issues and PRs. Things that involve typescript, such as typings and intellisense. status: awaiting response For issues and PRs. OP must respond (or change something, if it is a PR). Maintainers have no action labels Nov 19, 2021
@joe223
Copy link
Contributor Author

joe223 commented Nov 22, 2021

Could you please add a test to the types area of the repo? I think this would be a good place: https://github.com/sequelize/sequelize/blob/main/types/test/sequelize.ts

Prior to your change you should not have been able to do sequelize.cast(1, 'string') for example.

I'll add tests for sequelize.cast sequelize.fn and so on, to ensure these instance members do exist after types changed.

@github-actions github-actions bot removed the status: awaiting response For issues and PRs. OP must respond (or change something, if it is a PR). Maintainers have no action label Nov 22, 2021
@sdepold sdepold merged commit ae3cde5 into sequelize:main Nov 22, 2021
sdepold added a commit that referenced this pull request Dec 3, 2021
* feat(typescript): create alpha release with ts

* ci: add other ts versions to ci (#13686)

* fix: wrong interface used within mixin (#13685)

* fix(increment): fix key value broken query (#12985)

Co-authored-by: Sascha Depold <sdepold@users.noreply.github.com>

* fix(upsert): fall back to DO NOTHING if no update key values provided (#13594)

* fix(types): add instance member declaration (#13684)

* fix(types): add instance member declaration

* test(types): add static/instance members test cases

Co-authored-by: Sascha Depold <sdepold@users.noreply.github.com>

* Create aws-lambda.md (#12642)

* fix(docs): add aws-lamda route (#13693)

* fix(types): allow override json function with custom return type (#13694)

* fix(types): allow override to json function with custom return type

* fix(types): remove automatic linter changes

Co-authored-by: sander-mol <SMol@thepeoplegroup.nl>

* ci(docs): add doc generation to checks (#13704)

* docs: add logo (#13700)

* docs: add logo

* fix(docs): logo not show up (#13699)

* fix(build): markdownlint

* docs(readme): use internal link

* docs(index.md): use internal link

* docs(index): update logo rendering in docs

* Center logo and headline

Co-authored-by: Sascha Depold <sascha@depold.com>
Co-authored-by: Sascha Depold <sdepold@users.noreply.github.com>

* test: fix mocha (#13707)

Co-authored-by: Rik Smale <WikiRik@users.noreply.github.com>
Co-authored-by: Sascha Depold <sdepold@users.noreply.github.com>

* test: fix failing stack trace test (#13708)

* test: fix failing tests  (#13709)

* test: fix failing tests due to minification

Removes esbuild minification which was causing tests to fail and some changed behavior.

* Revert "fix(upsert): fall back to DO NOTHING if no update key values provided (#13594)"

This reverts commit 4071378.

* fix(upsert): fall back to DO NOTHING if no update key values provided (#13711)

* fix(upsert): fall back to DO NOTHING if no update key values provided (#13594)

* fix: remove erroneous .length in _.isEmpty

* refactor: use includes instead of or operators (#13706)

* docs(contributing): add section on adding/updating deps (#13715)

* docs(contributing): add Node versions

Fixes #13714

* docs(contributing): add section on adding/updating deps

* fix(types): add Col to where Ops (#13717)

* fix(types): add Col to where Ops

* fix(types): tests

* fix(data-types): unnecessary warning when getting data with DATE dataTypes (#13712)

* fix(data-types): unnecessary error when getting data with DATE dataTypes

* fix(data-types): date stringify mariadb

* fix(types): add missing schema field to sequelize options

Fixes #12606

Co-authored-by: Sascha Depold <sdepold@users.noreply.github.com>

* fix(example): fix coordinates format as per GeoJson (#13718)

* fix(example): fix coordinates format as per GeoJson

* Update data-types.js

Co-authored-by: Sascha Depold <sdepold@users.noreply.github.com>

* ci(node): use Node 16 instead of 12 (#13703)

* ci(node): add Node 14 and 16 to DB tests

* ci(node): move linting, test typing and release to Node 16

* ci(docs): use Node 16 for docs

* ci(node): run tests on Node 10 and 16

Co-authored-by: Rik Smale <WikiRik@users.noreply.github.com>

* refactor(postgres): move `clientMinMessages` from general to pg options (#13720)

* refactor(postgres): move `clientMinMessages` from general to pg options

* refactor(postgres): address review comments

* refactor(postgres): address review comments

* refactor(postgres): fix pipeline

* fix(dialect): try to fix flaky test

* fix(dialect): try to fix flaky test

Co-authored-by: Jesse Peng <jesse.peng@dynatrace.com>

* refactor(build): use rm instead of rmdir for Node 14 and up (#13702)

* fix(build): refactor rmdir to rm

* refactor(build): only use rm in Node 14 and up

* refactor(build): move consts inside function

* refactor(build): fix definition

Co-authored-by: Rik Smale <WikiRik@users.noreply.github.com>
Co-authored-by: Sascha Depold <sdepold@users.noreply.github.com>

* docs(postgres): warn about deprecated clientMinMessage option (#13727)

* docs(postgres): warn about deprecated clientMinMessage option

* docs(deprecation-warning): fix typo in deprecation warning

* docs(deprecation-warning): fix typo in deprecation warning

* meta(deps): upgrade (dev)deps (#13729)

Co-authored-by: Rik Smale <WikiRik@users.noreply.github.com>

* test(integration): mark and fix flaky test (#13735)

* feat(dialect): snowflake dialect support (#13406)

Co-authored-by: Mohamed El Mahallawy <mmahalwy@gmail.com>
Co-authored-by: bparan <bparan@softserveinc.com>
Co-authored-by: Matthew Blasius <matthew.blasius@expel.io>
Co-authored-by: WeRDyin <heyin223@gmail.com>
Co-authored-by: Marco Gonzalez <marcogrcr@gmail.com>
Co-authored-by: Constantin Metz <constantin@metzworld.com>
Co-authored-by: Sander Mol <Sandermol95@hotmail.com>
Co-authored-by: sander-mol <SMol@thepeoplegroup.nl>
Co-authored-by: Rik Smale <13023439+WikiRik@users.noreply.github.com>
Co-authored-by: Fauzan <fncolon@pm.me>
Co-authored-by: Rik Smale <WikiRik@users.noreply.github.com>
Co-authored-by: AllAwesome497 <47748690+AllAwesome497@users.noreply.github.com>
Co-authored-by: manish kakoti <madguy02@users.noreply.github.com>
Co-authored-by: Marco Kerwitz <marco@kerwitz.com>
Co-authored-by: Jesse Peng <vijcp@outlook.com>
Co-authored-by: Jesse Peng <jesse.peng@dynatrace.com>
@github-actions
Copy link
Contributor

github-actions bot commented Dec 4, 2021

🎉 This PR is included in version 6.12.0-beta.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions
Copy link
Contributor

🎉 This PR is included in version 6.12.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

aliatsis pushed a commit to creditiq/sequelize that referenced this pull request Jun 2, 2022
* fix(types): add instance member declaration

* test(types): add static/instance members test cases

Co-authored-by: Sascha Depold <sdepold@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released on @v6-beta released type: typescript For issues and PRs. Things that involve typescript, such as typings and intellisense.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants