Ability to type check setters in interface implementations

CraigMacomber commented 1 month ago

🔍 Search Terms

setter interface extends typecheck

✅ Viability Checklist

[X] This wouldn't be a breaking change in existing TypeScript/JavaScript code
[X] This wouldn't change the runtime behavior of existing JavaScript code
[X] This could be implemented without emitting different JS based on the types of the expressions
[X] This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, new syntax sugar for JS, etc.)
[X] This isn't a request to add a new utility type: https://github.com/microsoft/TypeScript/wiki/No-New-Utility-Types
[X] This feature would agree with the rest of our Design Goals: https://github.com/Microsoft/TypeScript/wiki/TypeScript-Design-Goals

⭐ Suggestion

Currently when implementing an interface, the type of the setter in the interface (when implicitly mutable or when given an explicit setter type) is not checked against the implementation. This means that an incorrect interface or an incorrect implementation can result in read-only or getter only properties getting assigned, and setters getting called with invalid typed.

I'm assuming this is working as intended due to the intentional not considering of readonly when computing assignability between types for legacy compatibility purposes.

This however doesn't prevent a strictness option from being added to allow projects to opt into having stricter checking of their interface implementations, which I think could be a compatible change.

I think this would align well with TypeScript's design goal #1: "Statically identify constructs that are likely to be errors.".

📃 Motivating Example

Playground link showing some code which has incorrect interface implementations which could error with more strict type checking.

Minimal example:

interface IFoo {
    x: number | string;
}
class Foo implements IFoo {
    get x(): number { return 5;}
    // No setter!
}
const f: IFoo = new Foo();
// This also allows assigning strings to our read only number.
f.x = "um...";

I would like a way to opt into having Foo emit a compile error that it does not correctly implement IFoo due to incorrect setter type.

The same issue can be reproduced, even when using TS 5.1's variant setter types:

// Even with unrelated getter and setter types, the setter type gets ignored when implementing the interface
interface IBaz {
    get x(): number;
    set x(value: string);
}

class Baz implements IBaz {
    get x(): number { return 5;}
    // No setter!
}

class IBaz2 implements IBaz {
    get x(): number{ return 5;}
    set x(value: object) {}
}

This case particularly seems like we should be able to give an error. All these cases compile without errors on latest TypeScript (5.5.3), even with all strictness flags.

Note: I find it interesting that splitting the interface into two interfaces then implementing both of them produces very different results (more errors than expected instead of less). Maybe thats a bug? Seems related.

💻 Use Cases

What do you want to use this for? Improving type safety for interface implanters. The specific concreate case motivating this is that Fluid Framewrok's tree has a schema system that generates base classes in a strongly typed way for users to extend. We had a customer declare their schema based class implements an interface which included mutable fields. These fields had a type more general than the one our tree implements, which is fine for reading the data out of the tree, but allowed invalid data to be provided to the setters. Due to https://github.com/microsoft/TypeScript/issues/43826 we can't actually declare our desired setter type, but even the type we can declare is not getting type checking for its interface implementation. Also if someone added a custom setter override (to work around the linked issue), they wouldn't get any type checking that they got the input type for it correctly aligned with the interface.
What shortcomings exist with current approaches? Compiler does not give errors for incorrect setters, which can result in errors going undetected and occurring at runtime. In some cases, this could result in data corruption.
What workarounds are you using in the meantime? Manually check that setter types are correct when authoring them and in code review, and hope our users do the same. Include additional runtime validation to help reduce risks of data corruption and instead produce informative runtime errors.

snarbies commented 1 month ago

jcalz commented 1 month ago

crosslinking to #53417 and #53594 (esp. "we always pick the read types")

microsoft / TypeScript