TypeScript icon indicating copy to clipboard operation
TypeScript copied to clipboard

Experiment with always using parameters from base types for derived methods

Open DanielRosenwasser opened this issue 6 years ago β€’ 16 comments

Note some related issues for doing this with properties rather than methods: #3667, #6118, #1373.

Goal

We want to make it easier for users to derive types without rewriting the same parameter signature every time

class Base {
  method(x: number) {
    // ...
  }
}

class Derived extends Base {
  method(x) {
    // 'x' should have the type 'number' here.
  }
}

Potential ideas

  • Only enable in noImplicitAny (doesn't work for default initializers 😞)
  • Revert to any in all locations, opt in with another strictness flag (😞)
  • Something else? πŸ˜•

Potential issues

Default initializers with more capable derived types

class A {
  a = 1;
}

class B extends A {
  b = 2;
}

class Base {
  method(x: A) {
    // ...
  }
}

class Derived extends Base {
  method(x = new B) {
    x.b;
    // Today, 'x' has type 'B' which is technically unsound
    // but that's just what we do. Does changing this to 'A' break things?
  }
}

Default initializers that become less-capable via contextual types


class Base {
  method(x: "a" | "b") {
    // ...
  }
}

class Derived extends Base {
  method(x = "a") {
    // We have to make sure 'x' doesn't have type '"a"'
    // which is both unsound and less useful.
  }
}

Distinction between properties and methods

Would this work?

class Base {
  method = (x: number) => {
    // ...
  }
}

class Derived extends Base {
  method(x) {
    // Does 'x' have the type 'number' here?
  }
}

What about this?

class Base {
  method(x: number) {
    // ...
  }
}

class Derived extends Base {
  method = (x) => {
    // Does 'x' have the type 'number' here?
  }
}

Keywords: base type derived contextual contextually inherit methods implicit any

DanielRosenwasser avatar May 04 '18 23:05 DanielRosenwasser

Couple of quick observations:

  1. Methods have the same implement/override ambiguity that properties do, as discussed in #6118. The core of any solution to both problems is the balancing of the majority who (I think) want to implement a method strictly against the twin minorities who alternately want to override a method, and those who want to implement a method, but unsoundly. I'm not convinced that restricting the domain to methods helps us choose the right tradeoffs, but it's a good idea to have a discussion since a couple of years have passed.
  2. I'm allergic to any function/method syntax distinctions unless everything else works out and there's no way around it. We should start with the assumption that there is no distinction between properties and methods.

sandersn avatar May 07 '18 16:05 sandersn

I'm allergic to any function/method syntax distinctions unless everything else works out and there's no way around it. We should start with the assumption that there is no distinction between properties and methods.

I agree - non-explicit syntactic switches are not something I enjoy.

weswigham avatar May 07 '18 17:05 weswigham

class Base {
  method = (x: number) => {
    // ...
  }
}

class Derived extends Base {
  method(x) {
    // Does 'x' have the type 'number' here?
  }
}

Conveniently, this code is illegal. The converse example would just be a case for #10570

RyanCavanaugh avatar May 16 '18 04:05 RyanCavanaugh

I really want to take this and #10570 as part of a "3.0 is a numerological excuse to make some good breaks" bundle.

Regarding this example

class Base {
  method(x: "a" | "b") {
    // ...
  }
}

class Derived extends Base {
  method(x = "a") {
    // We have to make sure 'x' doesn't have type '"a"'
    // which is both unsound and less useful.
  }
}

I think in any world, Derived#method has the signature (x: string): void. Parameter default expressions have to be widened, and we don't want to contextually type them (i.e. with a literal union type here) because that results in the type of a contextually-typed expression being observable (rules 1 and 2 of contextual typing: don't do that!). And it's certainly legal to write a derived type with a wider method signature than its base, so this code is already quite valid today and it'd be unwise to break it by invalidating anyone calling derived.method("hi")

RyanCavanaugh avatar May 16 '18 05:05 RyanCavanaugh

a "3.0 is a numerological excuse to make some good breaks" bundle

Interesting! Are there more breaks in this "bundle"? (Looking for example at https://github.com/Microsoft/TypeScript/pull/13971)

donaldpipowitch avatar Jul 20 '18 08:07 donaldpipowitch

Referencing a similar question regarding interfaces on SO

and complication on implementing multiple interfaces:

interface X { foo(i: string): string }
interface Y { foo(x: number): number }

class K implements X, Y {
  foo(x: number): number
  foo(x: string): string
  foo(x: number | string) {
    return x
  }
}

unional avatar Feb 03 '19 08:02 unional

I'm definitely curious about whether there's been any more progress on this?

At least from my perspective, it seems like inference of method parameters from the interface would be a pretty big ergonomics win (and happens relatively frequently, especially in library/framework code)

nevir avatar Feb 06 '19 18:02 nevir

Re:

  1. Methods have the same implement/override ambiguity that properties do, as discussed in #6118. The core of any solution to both problems is the balancing of the majority who (I think) want to implement a method strictly against the twin minorities who alternately want to override a method, and those who want to implement a method, but unsoundly. I'm not convinced that restricting the domain to methods helps us choose the right tradeoffs, but it's a good idea to have a discussion since a couple of years have passed.

Would a less strict take on it be more palatable? From what I can gather from the various issues/PRs towards this, the primary motivation seems to be improving the ergonomics, and is not so much about strictly checking classes to their parent types (we already have good assertions covering those cases).

For example: infer the types of method arguments when extending or implementsingβ€”but do not check compatibility when explicitly overriding a parameter?

E.g.

interface Base { doThing(a: number): number }

class Foo implements Base {
  doThing(a /* inferred to be number */) {
    return a;
  }
}

class Bar implements Base {
  // already an error here: 
  // "Property 'doThing' in type 'Bar' is not assignable to the same property in base type 'Base'."
  doThing(a: string /* not an error here */) {
    return parseInt(a);
  }
}

class Baz implements Base {
  doThing(a: 1 | 2 | 3 /* not widened to number*/) {
    return a;
  }
}

nevir avatar Apr 11 '19 21:04 nevir

Any movement on this issue?

nickofthyme avatar Jan 17 '20 23:01 nickofthyme

This much like a lot of very similar suggestions only consider implicit type behaviour changing fairly subtlety. #36165 suggests a way to explicitly say "the same type as expected by base class" which solves the issue of derive types without rewriting the same parameter signature every time.

tadhgmister avatar Jan 19 '20 22:01 tadhgmister

Copying and following upstream definitions is complicated as this: https://github.com/falsandtru/spica/blob/master/src/promise.ts

falsandtru avatar Jan 20 '20 17:01 falsandtru

@RyanCavanaugh wrote:

Regarding this example

class Base {
  method(x: "a" | "b") {
    // ...
  }
}

class Derived extends Base {
  method(x = "a") {
    // We have to make sure 'x' doesn't have type '"a"'
    // which is both unsound and less useful.
  }
}

I think in any world, Derived#method has the signature (x: string): void. Parameter default expressions have to be widened…

I don't see it that way. Per my long comment on #32082, type inheritance should use the nearest explicitly defined type from the ancestor classes, further narrowed by any interfaces implemented by the subclass. (See the linked comment for the full methodology and rationale.)

If you follow that logic, then Derived#method would have the same signature explicitly defined in superclass Base, which is method(x: "a" | "b") with an inferred return type of void. Parameter x should not simply take any or string β€” such a presumption forces each subclass to explicitly repeat the narrower inherited type, which is not DRY and indicates that type inheritance provides no value to method parameter types if they can be arbitrarily "widened" by default values. (And why would we widen the value to string and not to any or some other aribitrary type? All of these supersets of the explicit type "a" | "b" are arbitrary, and none of them represent correct type inheritance in classical OOP.)

My proposal for this issue of parameter types of overridden methods in derived classes follows the logic of my comment on #32082. Such methods should be allowed to specify explicit parameter types that are compatible with the class's ancestor classes and interfaces, but in the absence of explicit types, then the types should be inherited from those ancestor classes and interfaces as I proposed, and only inferred from default values if the ancestor classes and interfaces have nothing to say about this method name.

I really want to take this and #10570 as part of a "3.0 is a numerological excuse to make some good breaks" bundle.

This sentiment I agree with. πŸ˜„ So… how about TypeScript 4.0?

svicalifornia avatar Aug 02 '20 06:08 svicalifornia

Any updates on this?

darklight9811 avatar Jun 09 '21 01:06 darklight9811

Any updates on this?

haskelcurry avatar Sep 01 '22 06:09 haskelcurry

No Typescript expert but would love to see Typescript implement this somehow. There is also a gain in this when it comes to using module.js and module.d.ts separately, and I don't mean standalone modules or packages, smaller modules inside projects codebase.

Code example for more vibe:

// utility.d.ts

abstract class Utility {
  dirname(importMetaUrl: string): string;
  resolvePath(importMetaUrl: string, relativePath: string): string;

  // ...
}

declare const utility: Utility

export {utility, Utility}
// utility.js

import { URL, fileURLToPath } from 'url'
import path from 'path'

class Utility {
  /* Error: TS7006: Parameter 'importMetaUrl' implicitly has an 'any' type. */
  dirname(importMetaUrl) {
    const __filename = fileURLToPath(importMetaUrl)
    return path.dirname(__filename)
  }

  /* Error: TS7006: Parameter 'importMetaUrl' implicitly has an 'any' type. */
  /* Error: TS7006: Parameter 'relativePath' implicitly has an 'any' type. */
  resolvePath(importMetaUrl, relativePath) {
    return path.resolve(this.dirname(importMetaUrl), relativePath)
  }

  // ...
}

export const utility = new Utility()

muratgozel avatar Oct 07 '22 11:10 muratgozel

The JSDoc workaround is to use @type on the function itself:

class Joiner {
  /** @param {string[]} strings*/
  join(...strings) { return strings.join('') }
}

class CommaJoiner extends Joiner {
  /** @type {Joiner['join']} */
  join(...strings) {
    return strings.join(',');
  }
}

class NewLineJoiner extends Joiner {
  /** @type {Joiner['join']} */
  join(...strings) {
    return strings.join('\n');
  }
}

Though it would be nice to be able to use super['join'] since the above doesn't work when using mixins.

Not sure what kind of cast works on TS though. None of these do:

interface IJoiner {
  join: (...strings:string[]) => string;
};

class Joiner implements IJoiner {
  join(...strings:string[]) { return strings.join('') }
}

class CommaJoiner extends Joiner implements IJoiner {
  join(...strings){
    return strings.join(',');
  }
}

class NewLineJoiner extends Joiner {
  join!: Joiner['join'];

  join(...strings) {
    return strings.join('\n');
  }
}

The third one is the closest, but TS says it's a duplicate identifier without being able to ignore. Maybe relaxing is the key? If TS could also take hints from JSDocs that could be a solution.

clshortfuse avatar Nov 29 '22 00:11 clshortfuse

This is already possible:

class NewLineJoiner extends Joiner {
	join: Joiner['join'] = (...strings) => {
		return strings.join('\n')
	}
}

Despite using the arrow function, this supports this and super inside the override method, both in typings and in runtime.

IlyaSemenov avatar Dec 14 '22 11:12 IlyaSemenov

This is already possible:

class NewLineJoiner extends Joiner {
	join: Joiner['join'] = (...strings) => {
		return strings.join('\n')
	}
}

Despite using the arrow function, this supports this and super inside the override method, both in typings and in runtime.

look your memory app πŸ˜‹ you lost prototype reference and class optimisation for a type feature, this is not suitable. Type should not affect code base

jonlepage avatar Oct 19 '23 23:10 jonlepage

This is already possible:

class NewLineJoiner extends Joiner {
	join: Joiner['join'] = (...strings) => {
		return strings.join('\n')
	}
}

Despite using the arrow function, this supports this and super inside the override method, both in typings and in runtime.

look your memory app πŸ˜‹ you lost prototype reference and class optimisation for a type feature, this is not suitable. Type should not affect code base

then apply it to the arguments and return type:

class NewLineJoiner extends Joiner {
	join(...strings: Parameters<Joiner["join"]>): ReturnType<Joiner["join"]> {
		return strings.join('\n');
	}
}

playground link. And if that feels way to verbose and repetative then possibly support #36165.

tadhgmister avatar Oct 26 '23 02:10 tadhgmister

I think a first implementation could just cover the case when override is present to avoid various problems. A simple reference implementation could be this:

Whenever this pattern matches syntactically:

class MySubClass extends MyBaseClass {
       ...
	override myMethodName(arg1, ..., argN) {
		...
	}
	...
}

infer the types like this:

class MySubClass extends MyBaseClass {
       ...
	override myMethodName(arg1: Parameters<Joiner["join"]>[1], ..., argN: Parameters<Joiner["join"]>[N]): ReturnType<Joiner["join"]> {
		...
	}
	...
}

Of course this would have to be implemented in a more performant way.

hediet avatar Apr 26 '24 10:04 hediet