infinitered / apisauce

Axios + standardized errors + request/response transforms.
MIT License
2.78k stars 184 forks source link

Question about response type declaration #301

Open rnike opened 1 year ago

rnike commented 1 year ago

Hello, I have a question about response type, why is the ApiOkResponse has an optional data?

I suppose that if the request is successful, the ApiOkResponse<T> will contain ok:true with data:T,

if the request fail, response would become ApiErrorResponse with ok:false and data:undefined.

I am expecting the response type declaration would be

export interface ApiErrorResponse<T> {
  ok: false
  problem: PROBLEM_CODE
  originalError: AxiosError

- data?: T
+ data: undefined
  status?: number
  headers?: HEADERS
  config?: AxiosRequestConfig
  duration?: number
}
export interface ApiOkResponse<T> {
  ok: true
  problem: null
  originalError: null

- data?: T
+ data: T
  status?: number
  headers?: HEADERS
  config?: AxiosRequestConfig
  duration?: number
}

Is there any reason that the ApiOkResponse and ApiErrorResponse should have the same data type?

EricLiuZero commented 1 year ago

I got the same concern

jamonholmgren commented 1 year ago

Thanks for the question ... it is possible to return data along with your error, so we'd probably want to keep that. And it's possible to do a request that returns no data, so that's why we allow it to be optional. Might be a better way to type this, but keeping it flexible for those cases is important.

rnike commented 1 year ago

@jamonholmgren Thanks for the reply, I got your point, I've found the type declaration below we can define our own data type on success and failure

export type ApiResponse<T, U = T> = ApiErrorResponse<U> | ApiOkResponse<T>

but now I have another question, can we have fully control of our response type by removing the optional operator ? in this repo?

export interface ApiErrorResponse<T> {
  ok: false
  problem: PROBLEM_CODE
  originalError: AxiosError

- data?: T
+ data: T
  status?: number
  headers?: HEADERS
  config?: AxiosRequestConfig
  duration?: number
}
export interface ApiOkResponse<T> {
  ok: true
  problem: null
  originalError: null

- data?: T
+ data: T
  status?: number
  headers?: HEADERS
  config?: AxiosRequestConfig
  duration?: number
}
kartikeyvaish commented 1 year ago

@rnike , Did you find any solution for this?

Let's say I have a loginAPI function as shown

export function loginAPI(body: any): Promise<ApiResponse<SignInResponse, ErrorResponse>> {
    return apiSauceInstance.post('sign_in', body);
}

And my interfaces are defined as shown below

export interface SignInResponse {
    user: {
        name: string,
    }
}

export interface ErrorResponse {
    messages: {
        errors: {
            base: Array<string>
        },
        status_code: number
    }
}

So, after performing the API call, I have to explicitly check the ok parameter to be true or false to get Typescript Intellisense.

So,


if (apiResponse.ok === true) {     // <--- check explicitly for true
  console.log(apiResponse.data.user);
} else if (apiResponse.ok === false) {     // <--- check explicitly for false
  console.log(apiResponse.data.messages);
}

Below code doesn't give intellisense


if (apiResponse.ok) {     // <--- not checking for ok param to be true exactly
  console.log(apiResponse.data.user);
} else { 
  console.log(apiResponse.data.messages);     // <--- No intellisense
}
truongtv22 commented 1 year ago

@kartikeyvaish I also encountered the same problem =((