Skip to content

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.

*/
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