-
Notifications
You must be signed in to change notification settings - Fork 5
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
Can PhetioDynamicElementContainer.disposeElement be marked as public? #248
Comments
I also changed |
In general, is this a problem that will be fixed when we convert tandem to Typescript? RE Your commit: |
You are right and it would be better to have this marked as protected in the base class and public in the subclasses. However, I tried that again and confirmed that TypeScript doesn't understand it. It gives this error again:
Even though it knows class A {
// @protected
protected getName() {return 'a';}
}
class B extends A {
// @public
public getName() {return 'b';}
}
const b = new B();
b.getName(); OK to close? |
For the record, I also noticed this code doesn't type check: class A {
// @private
private getName() {return 'a';}
}
class B extends A {
// @public
public getName() {return 'b';}
}
const b = new B();
b.getName();
|
Should we put a TODO to mark this as |
I added a TODO and referenced back to this issue. Want to go on hold until then? |
This file is now in typescript, and disposeElement is marked as |
From phetsims/circuit-construction-kit-common#749, even though the overrides of disposeElement are marked as public, TypeScript used the parent annotation. But I was confused about why it is protected in the base class? Should it not be called typically? I'll mark it as public to make progress in phetsims/circuit-construction-kit-common#749, but would be good to discuss. Self assigning to take a closer look, but I'll consult with @zepumph soon.
The text was updated successfully, but these errors were encountered: