Option `strictMethodType`: `strictFunctionType` for method syntax. about typescript HOT 10 CLOSED

Assuming that enabling such an option doesn't break everything

This actually does break everything; it would make it roughly impossible to obtain an array or similar "actually invariant" object.

For example, you can't write

const arr: number[] = [];

because the RHS is never[], so it's illegal to push a number into it.

This can't be fixed with contextual typing of literals since there are other cases like:

// Correct program rejected --
// Invalid, RHS is number[]
const arr: unknown[] = [1, 2, 3].slice();

The underlying problem is there's no available distinction between these two methods:

class ArrayHolder {
  private arr: number[] = [];
  // Safe to covariantly alias the return of this function
  getCopy() {
    return arr.slice();
  }

  // Not safe to covariantly alias the return of this function
  getSelf() {
    return arr;
  }
}

Both are just "number[]" even though they have fundamentally different sound aliasing semantics.

For this flag to even be plausible, there'd need to be a reasonable way to solve this problem.

from typescript.

Assuming that enabling such an option doesn't break everything, having the option at least allows users to file issues and PRs on dependencies that don't work with it and work toward making the ecosystem compatible.

from typescript.

Have you some practical examples? I am curious about that.

So, you're presumably aware that Array has a push() method like

interface Array<T> {
  push(...items: T[]): number;
}

And this would be contravariant with your proposed flag, making Array<T> invariant in T . Then the following idiomatic TS code would break:

interface Animal { name: string }
function animalNames(animals: Animal[]) { return animals.map(x => x.name) }
const animals = [{ name: "Fido", age: 10 }, { name: "Snoopy", age: 5 }];
console.log(animalNames(animals));
// -------------------> ~~~~~~~

because even though every element of animals is an Animal, the array animals would not be an Animal[]. That is sound but would break lots of stuff. Indeed the standard way to avoid making a distributive conditional type is to wrap the type parameter in a one-tuple like

type DistributiveFoo<T> = T extends Bar ? Baz<T> : Qux;
type NonDistributiveFoo<T> = [T] extends [Bar] ? Baz<T> : Qux

And that would stop working and break all the things.

In the docs for --strictFunctionTypes it says:

During development of this feature, we discovered a large number of inherently unsafe class hierarchies, including some in the DOM. Because of this, the setting only applies to functions written in function syntax, not to those in method syntax.

So it's not just Array (does anyone have a DOM example to put in here)?

I checked This wouldn't be a breaking change [...] because I propose to gate this behind an option.

I love soundness but if I introduced a flag that broke everything when turned on, I don't think saying "it'll be off by default" would help much. This would be breaky.

Still, it makes sense for many projects to have this option enabled.

You apparently didn't get my point. Having the flag off by default doesn't mean it's not a breaking change. Anyone who wants to use your option would see breakage. Just having the option exist means that large amounts of TypeScript code (including the standard libraries provided with TypeScript itself) would need to change so that people who turn on the flag only see problems in their own code and not in a large chunk of the libraries they depend on. You're not proposing merely adding a flag; you're proposing adding a flag and rewriting a lot of TypeScript declarations. It really doesn't help if the only way for this not to be a breaking change is if the flag is off. We can do that right now by not introducing the flag.

And again, I'm not saying "this is a bad suggestion", I'm saying "this would be a breaking change". TypeScript does sometimes make breaking changes. Even --strictFunctionTypes was a breaking change. Breaking changes happen. I'm saying that we should be clear about the effects of introducing such a flag.

from typescript.

This wouldn't be a breaking change in existing TypeScript/JavaScript code

Anyone who turns on such a flag would undoubtedly see lots of breakage. Arrays are (unsoundly but conveniently) covariant in TypeScript and lots of code depends on that.

I love soundness but if I introduced a flag that broke everything when turned on, I don't think saying "it'll be off by default" would help much. This would be breaky.

from typescript.

Particularly since, because of how TS works, enabling the flag for your own project probably requires any dependencies you have to successfully compile against it, even if they don’t need it themselves. With options like this, in a lot of ways they’re only ”optional” in theory.

from typescript.

Anyone who turns on such a flag would undoubtedly see lots of breakage. Arrays are (unsoundly but conveniently) covariant in TypeScript and lots of code depends on that.

I checked This wouldn't be a breaking change [...] because I propose to gate this behind an option.

My guess is that this should be less of an issue now than it was 7 years ago when `strictFunctionType' was introduced.

Arrays are (unsoundly but conveniently) covariant in TypeScript and lots of code depends on that.

Have you some practical examples? I am curious about that.

I love soundness but if I introduced a flag that broke everything when turned on, I don't think saying "it'll be off by default" would help much. This would be breaky.

Still, it makes sense for many projects to have this option enabled.

Particularly since, because of how TS works, enabling the flag for your own project probably requires any dependencies you have to successfully compile against it, even if they don’t need it themselves. With options like this, in a lot of ways they’re only ”optional” in theory.

I am unsure to follow you. This could be checked at usage site? Similarly to strictFunctionType?

from typescript.

Most of its issues could be solved by using readonly arrays and carefully redesigning some function signatures (for example replacing T in includes by unknown or by a type parameter that extends T) in order to make readonly arrays covariant.

However, I accept that this could involve large breaking changes. Another possibility could create an exception for arrays. This is what Java is doing by making arrays covariant.

Given the current philosophy of TypeScript, I am no longer sure that this issue has any chance of being accepted.

from typescript.

@jcalz i'd love to see the breakage in my code. i've carefully avoided bivariance where possible, but i don't feel confident at all that i didn't accidentally mess up somewhere the compiler didn't want to check for me.

from typescript.

i've carefully avoided bivariance where possible

Your dependencies on bivariance are almost certainly implicit.

Out of the gate, the core lib is broken:

../../github/TypeScript/built/local/lib.es2015.generator.d.ts:21:11 - error TS2430: Interface 'Generator<T, TReturn, TNext>' incorrectly extends interface 'Iterator<T, TReturn, TNext>'.
  Types of property 'return' are incompatible.
    Type '(value: TReturn) => IteratorResult<T, TReturn>' is not assignable to type '(value?: TReturn | undefined) => IteratorResult<T, TReturn>'.
      Types of parameters 'value' and 'value' are incompatible.
        Type 'TReturn | undefined' is not assignable to type 'TReturn'.
          'TReturn' could be instantiated with an arbitrary type which could be unrelated to 'TReturn | undefined'.

21 interface Generator<T = unknown, TReturn = any, TNext = unknown> extends Iterator<T, TReturn, TNext> {
             ~~~~~~~~~

../../github/TypeScript/built/local/lib.es2018.asyncgenerator.d.ts:21:11 - error TS2430: Interface 'AsyncGenerator<T, TReturn, TNext>' incorrectly extends interface 'AsyncIterator<T, TReturn, TNext>'.
  Types of property 'return' are incompatible.
    Type '(value: TReturn | PromiseLike<TReturn>) => Promise<IteratorResult<T, TReturn>>' is not assignable to type '(value?: TReturn | PromiseLike<TReturn> | undefined) => Promise<IteratorResult<T, TReturn>>'.
      Types of parameters 'value' and 'value' are incompatible.
        Type 'TReturn | PromiseLike<TReturn> | undefined' is not assignable to type 'TReturn | PromiseLike<TReturn>'.
          Type 'undefined' is not assignable to type 'TReturn | PromiseLike<TReturn>'.

21 interface AsyncGenerator<T = unknown, TReturn = any, TNext = unknown> extends AsyncIterator<T, TReturn, TNext> {
             ~~~~~~~~~~~~~~

../../github/TypeScript/built/local/lib.es5.d.ts:331:11 - error TS2430: Interface 'CallableFunction' incorrectly extends interface 'Function'.
  Types of property 'apply' are incompatible.
    Type '{ <T, R>(this: (this: T) => R, thisArg: T): R; <T, A extends any[], R>(this: (this: T, ...args: A) => R, thisArg: T, args: A): R; }' is not assignable to type '(this: Function, thisArg: any, argArray?: any) => any'.
      The 'this' types of each signature are incompatible.
        Type 'Function' is not assignable to type '(this: any) => any'.
          Type 'Function' provides no match for the signature '(this: any): any'.

331 interface CallableFunction extends Function {
              ~~~~~~~~~~~~~~~~

../../github/TypeScript/built/local/lib.es5.d.ts:368:11 - error TS2430: Interface 'NewableFunction' incorrectly extends interface 'Function'.
  Types of property 'apply' are incompatible.
    Type '{ <T>(this: new () => T, thisArg: T): void; <T, A extends any[]>(this: new (...args: A) => T, thisArg: T, args: A): void; }' is not assignable to type '(this: Function, thisArg: any, argArray?: any) => any'.
      The 'this' types of each signature are incompatible.
        Type 'Function' is not assignable to type 'new () => any'.
          Type 'Function' provides no match for the signature 'new (): any'.

368 interface NewableFunction extends Function {
              ~~~~~~~~~~~~~~~

The DOM also has hundreds of errors because Events are not meant to be called from supertype aliases. It makes HTMLElement not assignable to Node due to addEventListener, so any DOM-related code gets broken quite hard.

This also breaks correct assumptions about array variance, specifically for readonly arrays. This code is sound by inspection:

interface MiniatureReadonlyArray<T> {
    getMutableCopy(): MiniatureMutableArray<T>;
    [i: number]: T;
}
interface MiniatureMutableArray<T> extends MiniatureReadonlyArray<T> {
    push(arg: T): void;
}
declare const sn_mini: MiniatureReadonlyArray<string | number>;
// Fails (wrong)
let snb_mini: MiniatureReadonlyArray<string | number | boolean> = sn_mini;

But it errors because from TS's perspective, a method returning an invariant type must make the entire type invariant.

We'd need a number of fairly invasive scaffolding features to make this work palatably. As-is, "just" making methods covariant creates a lot of incorrect errors.

You can try out this branch. https://github.com/RyanCavanaugh/TypeScript/tree/strictMethodTypes . It creates a new option "demoStrictMethodTypes"

from typescript.

Alternatively we could require explicit notation o make a parameter/return type bivariant.
For instance, we could mark a parameter as covariant (contravariant could be implied) and a return type as contravariant (covariance could be implied):

interface I {
  f(param: out Type): in boolean
}

I proposed a similar mechanism for Eiffel ten years ago.

from typescript.