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

cannot mixin and keep private properties #5070

Closed
paulsouche opened this Issue Oct 2, 2015 · 7 comments

Comments

Projects
None yet
5 participants
@paulsouche
Copy link

paulsouche commented Oct 2, 2015

Hi,

I'm wondering how to achieve this ?

class Test {
  public foo: string;
  private quux: string;
  // ... 
}   

class Plop {
  public bar: string;
  // ...
}

class TestPlopMixin implements Test, Plop {
  public foo: string;
  public bar: string;
  private quux: string;
  // Class TestPlopMixin incorrectly implements interface Test
  // Types have separate declarations of a private property 'quux'
}

Thanks

@kitsonk

This comment has been minimized.

Copy link
Contributor

kitsonk commented Oct 2, 2015

I am not 100% certain, but I think you have fallen down a bit of a rabbit hole. implements treats the classes as interfaces (and not as classes) and you cannot have private members of an interface (because they aren't part of the exposed API). I think the only way you can solve it, is remove the private from the Test class.

I think logically it works, even if it was class TestPlopMixin extends Test you would run into trouble if you declared another "private" variable with the same name in TestPlopMixin because the implementation of the ancestor class is expecting no one else to access it, because it is private... The compiler is keeping you from "stomping" on something, because in your implementation, you shouldn't be using something that someone has "reserved" for their private use. This comes in with the whole challenge in JavaScript that there is not really "private" properties, so I think the TypeScript compiler is trying to keep you from doing something "silly", though obviously the error message lacks a bit of clarity of why it is doing what it is doing.

@paulsouche

This comment has been minimized.

Copy link

paulsouche commented Oct 2, 2015

Thanks for the quick answer.

According to the doc TypeScript allow us to create mixins by creating classes that implements other classes.

I think this is already a bit "silly" for a non-JavaScript developer. So we cannot use this when an implemented class have private properties. Delete privates or make it public work but I don't think this is a good solution.

I'm OK with the idea that the mixin should not access the private of its ancestor but mixin objects that potentially expose "privates" properties is something daily for a JavaScript developer.

Personally I think that it should work without declaring the private on the class TestPlopMixin as an interface implementation that only declare public properties, keep it private in the class Test and just access the public API of the ancestor. But in this case it warns about the missing property.

@jbondc

This comment has been minimized.

Copy link
Contributor

jbondc commented Oct 2, 2015

I'd say this is a bug, the check happens here:
https://github.com/Microsoft/TypeScript/blob/master/src/compiler/checker.ts#L12869

What should happen in the context of implements classType is only check public members:
https://github.com/Microsoft/TypeScript/blob/master/src/compiler/checker.ts#L5141

Can likely submit a patch if TS team agrees.

@mhegazy

This comment has been minimized.

Copy link
Contributor

mhegazy commented Oct 2, 2015

What should happen in the context of implements classType is only check public members:

The current behavior is by design. private members will stick on the type. if you do not want the privates to appear, do not implement the class, implement an interface that is structurally equivalent to the class's public shape.

@paulsouche it is not possible to "implement" a class with a private member. the idea is that privates are unique to a specific class, and can not be accessed, redefined, or assigned to from outside the class. the compiler enforces this by making sure that there is one and only one declaration of a private, and that any implementation of that type has to have the same declaration. when you implement a class, you have to redefine the property, and that will violate the rule.

for mixin purposes you should not use private members in your class. if you must, create interfaces to represent the public shape of the class, which is what your implementation class will use, and the mixin function can then mixin a class implementing the same interface.

@mhegazy mhegazy added the Question label Oct 2, 2015

@paulsouche

This comment has been minimized.

Copy link

paulsouche commented Oct 3, 2015

Yeah indeed it works with an interface but I think it's weirder to implements both interfaces and classes. As the implementation of the mixin is up to me, I will only use interfaces that represent the classes.

Thanks for your time on this question.

@paulsouche paulsouche closed this Oct 3, 2015

@love2dishtech

This comment has been minimized.

Copy link

love2dishtech commented Jan 12, 2018

Although this is closed. This looks silly to me, the way we can implement mixins in any practical application using typescript does not make a lot of sense. Is there any plan to support mixins in a better way ?

@kitsonk

This comment has been minimized.

Copy link
Contributor

kitsonk commented Jan 12, 2018

@love2dishtech Since TypeScript 2.2 Mixin classes have been supported and allow private members. Is there something missing in that pattern for you?

@Microsoft Microsoft locked and limited conversation to collaborators Jul 3, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.