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

Override factory methods .of with extra mandatory parameters in subclasses. #1036

Open
Jym77 opened this issue Jan 21, 2022 · 0 comments
Open

Comments

@Jym77
Copy link
Contributor

Jym77 commented Jan 21, 2022

This is blocked until microsoft/TypeScript#4628 / microsoft/TypeScript#39699 has landed.

We make a heavy use of factory method public static of() in our classes. Unfortunately, TS doesn't let us change the signature in derived classes, even through our case is fairly safe. This has been a very discussed topic in the TS community, we should keep an eye on it and streamline code where possible.

Currently, instead of using:

class Foo {
  public static of(x: number): Foo {  }
}

class Bar extends Foo {
  // Rejected by TS due to incompatible signature of static function.
  public static of(x: number, y: number): Bar {  }
}

We use patterns like:

class Foo {
  public static of(x: number): Foo {  }
}

class Bar extends Foo {
  public static of(x: number, y: Option<number> = None): Bar {  }
}

In turn, this forces instances of Bar to store y as an option (or to have Bar.of possibly erroring at runtime without clean way of detecting cases where we don't provide y). Further down, users of Bar need some extra isSome checks when getting y, which clutters the full construction.

Another possible pattern, which keeps consumer simple but complexifies a bit the producer is

class Foo {
  public static of(x: number): Foo {  }
}

class Bar extends Foo {
  public static of(x: number): Foo
  public static of(x: number, y: number): Bar
  public static of(x: number, y?: number): Foo { 
    if (y !== undefined) {
      // do Bar specific stuff
    } else {
      return Foo.of(x)
    }
}

where the explicit overload fixes the situation.

A notable case for this is extended Diagnostic where all the extra data needs to be "optional" due to this. Sometimes we can provide a decent default (e.g. empty array), but in several cases, we need to wrap it in Option even though we know there will always be something.

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