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

Type inference for static Model methods #189

Merged
merged 3 commits into from
Nov 3, 2017

Conversation

schmod
Copy link
Contributor

@schmod schmod commented Nov 3, 2017

Adds type inference to most static methods on Model and its subclasses (create(), findAll(), etc) that previously required user-supplied generic parameters to correctly yield the correct return type.

Person.findAll<Person> simply becomes Person.findAll(), and TypeScript will automatically infer the correct return type. The old type signature will also continue to work. See microsoft/TypeScript#5863 for an explanation of what's going on under the hood that allows this to work.

New overloads have been added to Model.create<T extends Model<T>, A>(vals: A) : T and its cousins, so that Model.create<A>(vals: A) : T may also be used in as a shorthand for when developers want to provide strict typings for A, but don't need to override the automatic inference of T.

Updates tests to run against the current version of Sequelize v4, and uses newer Bluebird type definitions (a milder version of #166 until we can figure out what to do there -- necessary because the older version had an inaccurate definition of Promise.all).

Andrew Schmadel added 2 commits November 3, 2017 15:53
adds type inference to most static Model methods (create(), findAll(),
etc) that previously required user-supplied generic parameters to
correctly produce the correct return type.
@codecov-io
Copy link

codecov-io commented Nov 3, 2017

Codecov Report

Merging #189 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #189   +/-   ##
=======================================
  Coverage   96.85%   96.85%           
=======================================
  Files         100      100           
  Lines         921      921           
  Branches      125      125           
=======================================
  Hits          892      892           
  Misses          9        9           
  Partials       20       20
Impacted Files Coverage Δ
lib/annotations/Column.ts 91.66% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 57f044c...0cd0fb9. Read the comment docs.


/**
* Builds a new model instance and calls save on it.
*/
static create<T extends Model<T>>(values?: any, options?: ICreateOptions): Promise<T>;
static create<T extends Model<T>, A>(values?: A, options?: ICreateOptions): Promise<T>;
static create<T extends Model<T>>(this: (new () => T), values?: T, options?: ICreateOptions): Promise<T>;
Copy link
Member

Choose a reason for hiding this comment

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

It should be values?: any instead of values?: T

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed. I'm not sure how that got flipped! Will revise.

@RobinBuschmann
Copy link
Member

Hey @schmod really really good feature. Didn't know that this is possible. Thank you for implementing this. I only found one issue in the typings:
https://github.com/RobinBuschmann/sequelize-typescript/pull/189/files#diff-1c485f95a55d5ce0466f63a966ced297R306

@RobinBuschmann RobinBuschmann merged commit 581fd57 into sequelize:master Nov 3, 2017
@RobinBuschmann
Copy link
Member

0.6.0-beta.3 released

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

Successfully merging this pull request may close these issues.

3 participants