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

Suggestion: Change CircuitBreaker type declaration #846

Open
jeengbe opened this issue Feb 13, 2024 · 5 comments
Open

Suggestion: Change CircuitBreaker type declaration #846

jeengbe opened this issue Feb 13, 2024 · 5 comments

Comments

@jeengbe
Copy link

jeengbe commented Feb 13, 2024

I propose the following change to @types/opossum:

// From
declare class CircuitBreaker<TI extends unknown[] = unknown[], TR = unknown> extends EventEmitter { ... }

// To
declare class CircuitBreaker<T extends (...args: readonly never[]) => Promise<unknown>> extends EventEmitter { ... }

While this is the most breaking change, it makes a lot of work with the CircuitBreaker interface easier, for example:

export class CircuitBreakerThingRepository implements ThingRepository {
  private readonly circuitBreaker: CircuitBreaker<
    Parameters<ThingRepository['getThing']>,
    Awaited<ReturnType<ThingRepository['getThing']>>
  >;

  constructor(
    circuitBreakerFactory: CircuitBreakerFactory,
    delegate: ThingRepository,
  ) {
    this.circuitBreaker = circuitBreakerFactory.create(
      delegate.getThing.bind(delegate),
    );
  }

  getThing(arg: number): Promise<string> {
    return this.circuitBreaker.fire(arg);
  }
}

to

  private readonly circuitBreaker: CircuitBreaker<ThingRepository['getThing']>;

Practically, the type annotation is not required here (TS is able to infer it given the constructor assignment). Code such as:

export interface CircuitBreakerFactory {
  create<T extends (...args: readonly never[]) => unknown>(
    implementation: T,
  ): CircuitBreaker<Parameters<T>, Awaited<ReturnType<T>>>;
}

is also simpler written like:

export interface CircuitBreakerFactory {
  create<T extends (...args: readonly never[]) => Promise<unknown>>(
    implementation: T,
  ): CircuitBreaker<T>;
}

I am willing to submit a PR.

@lholmquist
Copy link
Member

submitting a PR would be helpful

@jeengbe
Copy link
Author

jeengbe commented Feb 21, 2024

See the above PR.

@jeengbe
Copy link
Author

jeengbe commented Mar 20, 2024

PR is ready to be merged, what's the status for this?

@lholmquist
Copy link
Member

The DefinetlyTyped repo is a community created repo and isn't associated with this repo, so you would need to ask those folks when it will be merged

Copy link
Contributor

This issue is stale because it has been open 30 days with no activity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants