take in generic for ManyQueryResponse

bwgjoseph commented 3 years ago

Hi,

Possible to accept generic for ManyQueryResponse?

createMany<Doc = T>(docs: Doc[] | Doc): Promise<ManyQueryResponse>;
removeMany<Doc = T>(filter?: LogicalWhereExpr<Doc>, options?: FindOptions): Promise<ManyQueryResponse>;
updateMany<Query = T>(filter?: LogicalWhereExpr<Query>, doc?: T | Partial<T>, options?: UpdateManyOptions): Promise<ManyQueryResponse>;

Notice the Promise<ManyQueryResponse> where we can pass return back Promise<ManyQueryResponse<T>> instead

This allows better typing like such

So that instead of using any[], we can return T[]

// instead of
export interface ManyResponse {
    data?: any[];
    match_number: number;
    success: number;
    errors: StatusExecution[];
}

// become
export interface ManyResponse<T> {
    data?: T[];
    match_number: number;
    success: number;
    errors: StatusExecution[];
}

And the data probably don't set it to optional? Seem like the response always return an empty array of data like such

{
         "message": {
          "data": []
           "errors": []
           "match_number": 0
           "success": 0
         }

Thanks!

AV25242 commented 3 years ago

Yeah data is intentional to be given out as opposed to an option

AV25242 commented 3 years ago

Issue has been created https://github.com/couchbaselabs/node-ottoman/issues/486

bwgjoseph commented 3 years ago

@AV25242 Can we fix data to make it non-optional first before the next beta?

AV25242 commented 3 years ago

you mean optional ? right now its non-optional

bwgjoseph commented 3 years ago

I mean the interface

// should be
export interface ManyResponse {
    // note, no ?
    data: any[];
    match_number: number;
    success: number;
    errors: StatusExecution[];
}

// instead of
export interface ManyResponse {
    data?: any[];
    match_number: number;
    success: number;
    errors: StatusExecution[];
}

Since that, all multi-methods will always return data, it doesn't make sense to make it optional unless there is case where data is not being returned

bwgjoseph commented 3 years ago

@AV25242 I was testing this with beta.2 but I'm not sure how to take advantage of the recent change to the generic type

So if I don't specify any types, then it gives me any which is correct

Same thing if I only specify the request type (doc)

So if I pass in the request, response type (doc, result), it will infer correctly

But then when I tries to access data, it will infer back to any

Any idea?

AV25242 commented 3 years ago

@bwgjoseph I am a bit lost here in general. What was this ticket created for and have we addressed whats needed for this ticket or did we derail this one ? :) if so can you assist cleaning this one up probably by opening another ticket and closing this one. This one seems to be overloaded if I am not wrong ? Or this one seems to be closer to the finish line (which I doubt ) ?

bwgjoseph commented 3 years ago

I think whatever was done did not resolve this issue, and that's why I'm trying to clarify.

Coming from a developer perspective, it seems like this wasn't done right because it is not inferring the correct type even when I pass in the type as seen in createMany<AirplaneInterface, AirplaneInterface> but when I try to use the response object, the type is lost and as seen in the last picture, it is now become any instead of the AirplaneInterface that I passed in

If done correctly, or at least what I think so, the data type should auto inferred as AirplaneInterface instead of any

Based on this PR https://github.com/couchbaselabs/node-ottoman/pull/506, I think something is still not done right that's why it's not inferred correctly. Hence, this issue is still not consider as resolved

gsi-alejandro commented 3 years ago

@AV25242 @bwgjoseph

@bwgjoseph was right, something was wrong. I declare the generics correctly but I forgot to pass this generic value into inherited components (extends QueryResponse<ManyResponse<T>>), therefore data was losing the type definition provided.

I already fixed it and will be ready to ship the next release.