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

Update mixinBehaviors annotation. Behaviors don't satisfy PolymerInit. #5036

Merged
merged 1 commit into from
Jan 11, 2018

Conversation

aomarks
Copy link
Member

@aomarks aomarks commented Jan 10, 2018

Behaviors don't have an is, so this type annotation was slightly off. I think what we really want is something like PolymerInit minus the is, but for now this at least makes things compile.

@aomarks aomarks requested a review from dfreedm January 10, 2018 23:19
Copy link
Contributor

@TimvdLippe TimvdLippe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would the intersection of PolymerInit and {is?: undefined} work maybe?

@aomarks
Copy link
Member Author

aomarks commented Jan 11, 2018

Would the intersection of PolymerInit and {is?: undefined} work maybe?

We don't have a way to express that in Closure, and it doesn't seem to work in TypeScript anyway (I think the stricter is wins in the intersection). I think we'd need to just write two interfaces or something.

@justinfagnani
Copy link
Contributor

it doesn't seem to work in TypeScript anyway (I think the stricter is wins in the intersection)

An intersection of types has unions of shared members, since the type has to satisfy both interfaces.

@justinfagnani
Copy link
Contributor

Can you make is optional on PolymerInit?

@aomarks
Copy link
Member Author

aomarks commented Jan 11, 2018

Can you make is optional on PolymerInit?

It's maybe not optional in some cases, and there might be code that would have to be updated to check for undefined to make Closure happy. So I think we just need two interfaces.

For now just going to revert this back to object, which is what we had before.

@aomarks aomarks merged commit d7ea246 into master Jan 11, 2018
@aomarks aomarks deleted the mixin-behavior-annotation branch January 11, 2018 00:30
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.

4 participants