Skip to content

When checking if two class types are assignable, don't check the assignability of their private member variables #49144

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

Closed
5 tasks done
vanchagreen opened this issue May 17, 2022 · 3 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@vanchagreen
Copy link

Suggestion

🔍 Search Terms

private member variables
assignability
contravariant

✅ Viability Checklist

My suggestion meets these guidelines:

  • This wouldn't be a breaking change in existing TypeScript/JavaScript code
  • This wouldn't change the runtime behavior of existing JavaScript code
  • This could be implemented without emitting different JS based on the types of the expressions
  • This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, new syntax sugar for JS, etc.)
  • This feature would agree with the rest of TypeScript's Design Goals.

⭐ Suggestion

When checking if two class types are assignable, don't check the assignability of their private member variables.

📃 Motivating Example

class Bag<T> {
    constructor(private fn: (a: T) => any) {}
}

let numberBag : Bag<number>;
let unionBag: Bag<number | string>;

unionBag = numberBag; // invalid, because 'string' is not assignable to 'number'

playground link

I would expect the assignment above to be safe, even though Function types are contravariant in their arguments, because the fn member variable is private, and the inner Bag code only has access to an arbitrary T.

💻 Use Cases

The most common use-case I've run into is wanting to have a function in the client code that takes Bag<unknown> as a parameter:

function clientFn(bag: Bag<unknown>) {}

clientFn(numberBag); // invalid

My intent here is for the clientFn to explicitly not care about the inner T. Note that this could be fixed by using any, and while I don't like any in general, maybe this could be a good time for it?

Another use-case might be where client code wanted to create a list of Bags, where the Ts are constrained to an allowable set:

// continued from above

type AllowedBagTypes = number | string;

const bags : Array<Bag≤AllowedBagTypes>> = [];
bags.push(numberBag);

Note, this can be fixed by changing the AllowedBagTypes type to Bag<number> | Bag<string>.

One final thing to note is that this Bag class breaks the client code only because of the private fn member variable. That means that adding such a variable to an otherwise working existing class could unexpectedly break client code.


I'm not 100% sure this is actually a safe suggestion, and I'd be very happy to be corrected if I'm wrong! I think most of the use-cases can be fixed by using any or by changing the types in the client code around, so this may not be a super valuable suggestion for the effort. But just figured I'd put it out there!

@jcalz
Copy link
Contributor

jcalz commented May 17, 2022

Duplicate of or very strongly related to #18499

Private members are visible to other instances of the same class. If they didn't affect compatibility, things would explode.

class Bag<T> {
    constructor(private fn: (a: T) => any) { }
    foo() {
        const stringBag: Bag<string> = this; // error, for good reason
        //    ~~~~~~~~~ <-- Type 'Bag<T>' is not assignable to type 'Bag<string>'.
        stringBag.fn("oopsie");
    }
}

new Bag((x: number) => x.toFixed(1)).foo() // 💥 runtime error! x.toFixed is not a function

Playground link to code

@vanchagreen
Copy link
Author

Ahhh I didn't realize that private members are visible to other instances of the same class. I thought there would be some reason why my idea was flawed :) Thanks!

@RyanCavanaugh RyanCavanaugh added the Working as Intended The behavior described is the intended behavior; this is not a bug label May 18, 2022
@typescript-bot
Copy link
Collaborator

This issue has been marked 'Working as Intended' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests

4 participants