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

always emit clutz types in heritage clauses #1081

Merged
merged 1 commit into from
Sep 23, 2019
Merged

always emit clutz types in heritage clauses #1081

merged 1 commit into from
Sep 23, 2019

Conversation

evmar
Copy link
Contributor

@evmar evmar commented Sep 20, 2019

When there is a type-value conflict, e.g.
interface B {}
const B = 3;
then tsickle doesn't emit a Closure type for the interface.

Then when
class A implements B
tsickle omits the @implements for B, because B doesn't exist.

But when B comes from a Clutz definition, we know that the type
B does exist on the Closure side, and we must emit the @implements.

This change keys off a Clutz-emitted file header to control this.

Partial fix of #1072.

@evmar evmar requested review from rkirov and mprobst and removed request for rkirov September 20, 2019 22:00
@evmar
Copy link
Contributor Author

evmar commented Sep 20, 2019

Depends on angular/clutz#926

src/type_translator.ts Outdated Show resolved Hide resolved
When there is a type-value conflict, e.g.
  interface B {}
  const B = 3;
then tsickle doesn't emit a Closure type for the interface.

Then when
  class A implements B
tsickle omits the @implements for B, because B doesn't exist.

But when B comes from a Clutz definition, we know that the type
B does exist on the Closure side, and we must emit the @implements.

This change keys off a Clutz-emitted file header to control this.

Partial fix of #1072.
@evmar
Copy link
Contributor Author

evmar commented Sep 23, 2019

Post-review note: I had to add the underlying goog.module that we're simulating Clutz for here for the test that verifies Closure compilation to pass. It's just a straightforward JS file, so I think it's harmless to submit.

@evmar evmar merged commit ef80516 into angular:master Sep 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants