From 548379ca2a6fe81709ab3deab2d081d1282c0179 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Thu, 28 Jun 2018 12:59:13 +0200 Subject: [PATCH 1/2] Fix type definitions for PersistedModel API Callback's first argument is an optional Error now. Was: required `any`. PersistedModel methods return `PersistedModel` now. Before this change, methods were returning `PersistedData` (`PersistedModel | AnyObject`). The problem with `AnyObject` is that it does not contain any `PersistedModel` instance data and cannot be assigned to functions expecting `Partial`. As a result, consumers of this API were forced to either cast the result to `PersistedModel` (which feels wrong) or deal with the `AnyObject` case (which never happen at runtime). Fix definition of `ModelData` to `T | Partial`. Before this change, `ModelData` allowed any values not related to the actual model at all, for example arrays. --- types/common.d.ts | 2 +- types/model.d.ts | 2 +- types/persisted-model.d.ts | 56 +++++++++++++++++++++----------------- 3 files changed, 33 insertions(+), 27 deletions(-) diff --git a/types/common.d.ts b/types/common.d.ts index 3d31389d..5831b144 100644 --- a/types/common.d.ts +++ b/types/common.d.ts @@ -18,7 +18,7 @@ export type Options = AnyObject; /** * Type alias for Node.js callback functions */ -export type Callback = (err: any, result?: T) => void; +export type Callback = (err?: Error | null, result?: T) => void; /** * Return export type for promisified Node.js async methods. diff --git a/types/model.d.ts b/types/model.d.ts index 18938ea6..90af3816 100644 --- a/types/model.d.ts +++ b/types/model.d.ts @@ -246,4 +246,4 @@ export declare class ModelBuilder extends EventEmitter { * Union export type for model instance or plain object representing the model * instance */ -export type ModelData = T | AnyObject; +export type ModelData = T | Partial; diff --git a/types/persisted-model.d.ts b/types/persisted-model.d.ts index c5442c93..cb99617a 100644 --- a/types/persisted-model.d.ts +++ b/types/persisted-model.d.ts @@ -38,8 +38,14 @@ export declare class PersistedModel extends ModelBase { static create( data: PersistedData, options?: Options, - callback?: Callback, - ): PromiseOrVoid; + callback?: Callback, + ): PromiseOrVoid; + + static create( + data: PersistedData[], + options?: Options, + callback?: Callback, + ): PromiseOrVoid; /** * Update or insert a model instance @@ -51,20 +57,20 @@ export declare class PersistedModel extends ModelBase { static upsert( data: PersistedData, options?: Options, - callback?: Callback, - ): PromiseOrVoid; + callback?: Callback, + ): PromiseOrVoid; static updateOrCreate( data: PersistedData, options?: Options, - callback?: Callback, - ): PromiseOrVoid; + callback?: Callback, + ): PromiseOrVoid; static patchOrCreate( data: PersistedData, options?: Options, - callback?: Callback, - ): PromiseOrVoid; + callback?: Callback, + ): PromiseOrVoid; /** * Update or insert a model instance based on the search criteria. @@ -86,15 +92,15 @@ export declare class PersistedModel extends ModelBase { where: Where, data: PersistedData, options?: Options, - callback?: Callback, - ): PromiseOrVoid; + callback?: Callback, + ): PromiseOrVoid; static patchOrCreateWithWhere( where: Where, data: PersistedData, options?: Options, - callback?: Callback, - ): PromiseOrVoid; + callback?: Callback, + ): PromiseOrVoid; /** * Replace or insert a model instance; replace existing record if one is found, @@ -110,8 +116,8 @@ export declare class PersistedModel extends ModelBase { static replaceOrCreate( data: PersistedData, options?: Options, - callback?: Callback, - ): PromiseOrVoid; + callback?: Callback, + ): PromiseOrVoid; /** * Finds one record matching the optional filter object. If not found, creates @@ -147,8 +153,8 @@ export declare class PersistedModel extends ModelBase { filter: Filter, data: PersistedData, options?: Options, - callback?: Callback, - ): PromiseOrVoid; + callback?: Callback, + ): PromiseOrVoid; /** * Check whether a model instance exists in database. @@ -183,7 +189,7 @@ export declare class PersistedModel extends ModelBase { filter?: Filter, options?: Options, callback?: Callback, - ): PromiseOrVoid; + ): PromiseOrVoid; /** * Find all model instances that match `filter` specification. @@ -215,8 +221,8 @@ export declare class PersistedModel extends ModelBase { static find( filter?: Filter, options?: Options, - callback?: Callback, - ): PromiseOrVoid; + callback?: Callback, + ): PromiseOrVoid; /** * Find one model instance that matches `filter` specification. @@ -246,8 +252,8 @@ export declare class PersistedModel extends ModelBase { static findOne( filter?: Filter, options?: Options, - callback?: Callback, - ): PromiseOrVoid; + callback?: Callback, + ): PromiseOrVoid; /** * Destroy all model instances that match the optional `where` specification. @@ -362,8 +368,8 @@ export declare class PersistedModel extends ModelBase { id: any, data: PersistedData, options?: Options, - callback?: Callback, - ): PromiseOrVoid; + callback?: Callback, + ): PromiseOrVoid; /** * Return the number of records that match the optional "where" filter. @@ -479,8 +485,8 @@ export declare class PersistedModel extends ModelBase { */ reload( options?: Options, - callback?: Callback, - ): PromiseOrVoid; + callback?: Callback, + ): PromiseOrVoid; /** * Set the correct `id` property for the `PersistedModel`. Uses the `setId` method if the model is attached to From 6f3675b13ccef8bd0d628e6ce8588cec6528cf48 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Fri, 29 Jun 2018 08:35:22 +0200 Subject: [PATCH 2/2] fixup! address code review comments --- types/common.d.ts | 2 +- types/model.d.ts | 8 +++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/types/common.d.ts b/types/common.d.ts index 5831b144..912076c9 100644 --- a/types/common.d.ts +++ b/types/common.d.ts @@ -18,7 +18,7 @@ export type Options = AnyObject; /** * Type alias for Node.js callback functions */ -export type Callback = (err?: Error | null, result?: T) => void; +export type Callback = (err?: any | null, result?: T) => void; /** * Return export type for promisified Node.js async methods. diff --git a/types/model.d.ts b/types/model.d.ts index 90af3816..6e525071 100644 --- a/types/model.d.ts +++ b/types/model.d.ts @@ -242,8 +242,14 @@ export declare class ModelBuilder extends EventEmitter { ): ModelBaseClass; } +/** + * An extension of the built-in Partial type which allows partial values + * in deeply nested properties too. + */ +export type DeepPartial = { [P in keyof T]?: DeepPartial; }; + /** * Union export type for model instance or plain object representing the model * instance */ -export type ModelData = T | Partial; +export type ModelData = T | DeepPartial;