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

Warn on unsupported class members #461

Conversation

samwgoldman
Copy link
Member

Before this change, flow would silently ignore these properties and
instead users would see "Property not found" errors when they went to
use the desired properties.

I intentionally didn't include a fallthrough case, because the compiler
will warn if this pattern ever becomes inexhaustive in the future.
However, that's currently just a warning. Can we make it an error?

Also, because of the way the AST is defined, some of these cases are
actually syntax errors, like computed- and literal-key method bodies. I
would be happy to also make the changes to the AST to restrict the
representation to exclude impossible cases, if that seems important.

See #444

Before this change, flow would silently ignore these properties and
instead users would see "Property not found" errors when they went to
use the desired properties.

I intentionally didn't include a fallthrough case, because the compiler
will warn if this pattern ever becomes inexhaustive in the future.
However, that's currently just a warning. Can we make it an error?

Also, because of the way the AST is defined, some of these cases are
actually syntax errors, like computed- and literal-key method bodies. I
would be happy to also make the changes to the AST to restrict the
representation to exclude impossible cases, if that seems important.
@mroch
Copy link
Contributor

mroch commented May 22, 2015

I intentionally didn't include a fallthrough case, because the compiler
will warn if this pattern ever becomes inexhaustive in the future.
However, that's currently just a warning. Can we make it an error?

fd23321 :)

@samwgoldman
Copy link
Member Author

@mroch is on 🔥!

@ghost ghost closed this in 0b4d04d May 26, 2015
@gabelevi
Copy link
Contributor

Awesome, thanks @samwgoldman!

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants