Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Question about response type declaration #301

Open
rnike opened this issue Dec 13, 2022 · 5 comments
Open

Question about response type declaration #301

rnike opened this issue Dec 13, 2022 · 5 comments

Comments

@rnike
Copy link

rnike commented Dec 13, 2022

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
Copy link

I got the same concern

@jamonholmgren
Copy link
Member

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
Copy link
Author

rnike commented Dec 20, 2022

@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
Copy link

kartikeyvaish commented Aug 6, 2023

@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
Copy link

@kartikeyvaish I also encountered the same problem =((

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

No branches or pull requests

5 participants