Skip to content
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

Erase non-exported base classes #417

Closed
eladb opened this issue Apr 1, 2019 · 0 comments · Fixed by #425
Closed

Erase non-exported base classes #417

eladb opened this issue Apr 1, 2019 · 0 comments · Fixed by #425
Assignees
Labels
effort/small Small work item – less than a day of effort

Comments

@eladb
Copy link
Contributor

eladb commented Apr 1, 2019

If an exported class extends a non-exported class, it should be possible to erase the non-exported class from the public API by copying it's interface to the derived class. This is allowed in TypeScript and there is no reason it won't be allowed in jsii.

The compiler should respect any public bases of the erased class and copy those as well to the derived class.

This is already supported for interfaces.

Repro:

export class PublicBaseOfBase {
  public foo() { }
}

class PrivateBase extends PublicBaseOfBase {
  public goo() { }
}

export class Derived extends PrivateBase {
  public noo() { }
}

After erasure, the API of Derived should be:

class Derived extends PublicBaseOfBase {
  public goo() { }
  public noo() { }
}
RomainMuller added a commit that referenced this issue Apr 3, 2019
When an exported class extends an un-exported class, merge the declarations
of the un-exported base into the exported one, and replace the base class
relationship with the base's own base (recursively until an exported base is
found, or no further base class is declared - in which case the exported class
will not have a base class at all).

Declarations from the erased base class(es) will be ignored if there is another
declaration with the same name and type in the exported type (or another
erased base class that is closer to the exported type in the inheritance chain).

Fixes #417
RomainMuller added a commit that referenced this issue Apr 3, 2019
The previous version of interface erasure simply replaced un-exported
interfaces with whatever their exported bases were (if any), but did not
merge declarations of un-exported types into the exported ones.

This merges declarations from the erased interfaces into the exported
interface, and ignores any re-declaration that is hidden by an existing
declaration in the exported type, or another erased interface.

Fixes #417
@RomainMuller RomainMuller self-assigned this Apr 3, 2019
@RomainMuller RomainMuller added the effort/small Small work item – less than a day of effort label Apr 3, 2019
RomainMuller added a commit that referenced this issue Apr 3, 2019
…se (#425)

When an exported class extends an un-exported class, merge the declarations
of the un-exported base into the exported one, and replace the base class
relationship with the base's own base (recursively until an exported base is
found, or no further base class is declared - in which case the exported class
will not have a base class at all).

Declarations from the erased base class(es) will be ignored if there is another
declaration with the same name and type in the exported type (or another
erased base class that is closer to the exported type in the inheritance chain).

Fixes #417
RomainMuller added a commit that referenced this issue Apr 4, 2019
…those (#426)

The previous version of interface erasure simply replaced un-exported
interfaces with whatever their exported bases were (if any), but did not
merge declarations of un-exported types into the exported ones.

This merges declarations from the erased interfaces into the exported
interface, and ignores any re-declaration that is hidden by an existing
declaration in the exported type, or another erased interface.

Fixes #417
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort/small Small work item – less than a day of effort
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants