bwgjoseph / mongoose-vs-ottoman

feature comparison between mongoose and ottoman
0 stars 1 forks source link

overload function declaration (API) #69

Closed bwgjoseph closed 3 years ago

bwgjoseph commented 3 years ago

Hi,

With the recent changes to generics, specifically to https://github.com/couchbaselabs/node-ottoman/pull/449. I noticed a break in my application types.

In short, I have something like

class Post<T = any> {
    async _get(id: string, params: Params = {}): Promise<T> {
        const { query } = this.filterQuery(params);
        // Model here refers to ottoman model
        return this.Model.findById(id, query);
    }

    async _update(id: string, data: T, params: Params = {}): Promise<T> {
        return this.Model.replaceById(id, data);
    }
}

But due to the recent change, the API is defined as such

findById<Result = R>(id: string, options?: FindByIdOptions): Promise<Document<Result> | Result>;
replaceById<Doc = T, Result = R>(id: string, data: Doc | Document<Doc>, options?: MutationFunctionOptions): Promise<Result | Document<Result>>;

And it will complain

image

image

May I suggest switching over to use function-overloads like such

findById<Result = R>(id: string, options?: FindByIdOptions): Promise<Result>;
findById<Result = R>(id: string, options?: FindByIdOptions): Promise<Document<Result>>;

replaceById<Doc = T, Result = R>(id: string, data: Doc, options?: MutationFunctionOptions): Promise<Result>;
replaceById<Doc = T, Result = R>(id: string, data: Document<Doc>, options?: MutationFunctionOptions): Promise<Document<Result>>;

I'm not entirely sure if I missed out on anything else at this point, but I think this should do it.

There are more other APIs changed, above are just 2 examples I listed.

ottoman alpha.28

AV25242 commented 3 years ago

Have created issues to investigate

bwgjoseph commented 3 years ago

Just a note that this is not only limited to findById, removeById API

httpJunkie commented 3 years ago

Fixed w/ alpha 29