AlexGilleran / jsx-control-statements

Neater If and For for React JSX
https://www.npmjs.com/package/babel-plugin-jsx-control-statements
MIT License
1.62k stars 64 forks source link

FlowTyped Declarations #73

Open colshacol opened 6 years ago

colshacol commented 6 years ago

In usage with Flow, I get errors like this:

screen shot 2018-01-09 at 12 14 30 am

I am wondering that, since the values passed into condition are negated to truthy or falsey, shouldn't it accept any type?

Apologies if this question belongs elsewhere. If so, please do direct me.

colshacol commented 6 years ago

Current declarations:

screen shot 2018-01-09 at 12 17 29 am

What I believe it should be:

screen shot 2018-01-09 at 12 17 42 am
AlexGilleran commented 6 years ago

Ehhhh I see your point but I actually like that it forces you to put the !! in front and say "I definitely want to check for truthiness here, it's not that I've just got the wrong variable/property".

You can just use your own definitions in place of the ones supplied right?

colshacol commented 6 years ago

@AlexGilleran Apologies, I am constantly at 200% capacity and forgot to take care of this issue. -- Yeah, I was able to patch it up without any issues.

I think it may boil down to opinion on this, but thinking about what I've used, personally, as well as what I have seen in the community, it is the norm to follow conventions such as

if (foo) { ... }

or

foo && whatever;

etc...

In places that values are negated by default, I don't see many explicitly negating.

JalilArfaoui commented 6 years ago

Hello @AlexGilleran

I agree it is a matter of opinion if we should or not have an explicit boolean cast in conditions.

Except that I think, like @colshacol, that we should have a consistent behavior between if (foo) and <If condition={foo}>.

So, since Flow does not raise an error on if statement with non-boolean value, I think this link should not either.

https://flow.org/try/#0PQKgBAAgZgNg9gdzCYAoVUCuA7AxgFwEs5swo44AKADwC4wBnfAJ0OwHMBKep1jsAN6owYQlDA1Og4SLDMApvkzNS1ANwyAvjIVKVYAEQATeVACGmGPkYs27Axu1A

draperunner commented 6 years ago

I agree @colshacol and @JalilArfaoui . Since it is so obvious that whatever passed into <If condition={}> will be treated as a boolean, like a normal if statement in JavaScript, I think it should be typed as any.

In my project we end up casting all statements with Boolean() like this: <If condition={Boolean(...)}>, which I think is just noisy.