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

Convert KyResponse to an interface for better extensibility #643

Open
yslpn opened this issue Oct 6, 2024 · 0 comments
Open

Convert KyResponse to an interface for better extensibility #643

yslpn opened this issue Oct 6, 2024 · 0 comments

Comments

@yslpn
Copy link

yslpn commented Oct 6, 2024

Convert KyResponse to an interface for better extensibility

Current Behavior

Currently, KyResponse is defined as a type in the ky library. This makes it difficult to extend or modify the type when users need to add custom properties or methods.

Proposed Change

Convert KyResponse from a type to an interface. This change would allow users to extend the interface as needed without encountering type conflicts.

Problem Description

When trying to extend the KyResponse type, users encounter type errors. For example:

declare module "ky" {
  interface HTTPError<T = unknown> {
    response: KyResponse<T> & { data: T };
  }
}

export const apiKy = ky.create({ prefixUrl: PUBLIC_API_URL, credentials: "include" }).extend({
  hooks: {
    beforeError: [
      async (error) => {
        const { response } = error;
        if (response) {
          error.response.data = await response.json();
        }

        return error;
      },
    ],
  },
});

This results in the following error:

Subsequent property declarations must have the same type. Property 'response' must be of type 'KyResponse<T>', but here has type '{ json: <J = T>() => Promise<J>; } & Response & { data: T; }'.

Attempting to redefine KyResponse as an interface leads to a different error:

declare module "ky" {
  interface KyResponse<T = unknown> extends Response {
    json: <J = T>() => Promise<J>;
    data: T;
  }
}

Error:

Duplicate identifier 'KyResponse'.ts(2300)
index.d.ts(10, 15): 'KyResponse' was also declared here.

Proposed Solution

Change the definition of KyResponse from a type to an interface in the ky library:

interface KyResponse<T = unknown> extends Response {
  json: <J = T>() => Promise<J>;
}

This change would allow users to extend the interface as needed:

declare module "ky" {
  interface KyResponse<T = unknown> {
    data: T;
  }
}

Benefits

  1. Improved extensibility for users who need to add custom properties or methods to KyResponse.
  2. Better TypeScript support for custom extensions to the ky library.
  3. Maintains backwards compatibility while providing more flexibility.

Potential Drawbacks

  1. Slight change in how KyResponse is defined, which might require updates to documentation.
  2. Possible need for minor adjustments in the library's internal implementation.

Additional Context

This change would be particularly helpful for users who need to add custom error handling or data processing to their ky-based API calls.

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

1 participant