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

RFC: Remove <If> #12

Merged
merged 1 commit into from Apr 2, 2019
Merged

RFC: Remove <If> #12

merged 1 commit into from Apr 2, 2019

Conversation

leebyron
Copy link
Owner

@leebyron leebyron commented Apr 2, 2019

I think <If> and <Else> might be a distraction from the value of <For> and I'm considering remove it. They're also potentially helpful for clearer syntax compared to {condition && <Node />} but have some real drawbacks as well.

Perhaps we should consider including them in a react-conditionals library?

@leebyron
Copy link
Owner Author

leebyron commented Apr 2, 2019

cc @manuelbieh - Presumably <Choose> would live with <If>?

@superhawk610
Copy link

superhawk610 commented Apr 2, 2019

I believe some of the syntactic clarity <If> aims to bring is lost in the closing tags inherent in XML/JSX structure. In traditional code this isn't an issue since } isn't visually intrusive. The nested structure is also a bit distracting. Compare the following:

if (condition) {
  // do something
} else if (anotherCondition) {
  // do something else
} else {
  // do another thing
}
<If test={condition}>
  {/* do something */}
  <ElseIf test={anotherCondition}>
    {/* do something else */}
    <Else>
      {/* do another thing */}
    </Else>
  </ElseIf>
</If>

Traditional JS blocks are visually sparse and shift focus to their contents, while XML tag blocks are a bit more visually weighty and thus distracting.

The only real drawback of guards ({condition && <Element />}) is they will render the string value of condition (eg - 'false') in some contexts. However, this is rather trivially solved by using the slightly longer ternary form {condition ? <Element /> : null}. Are there any other drawbacks to this format?

@leebyron
Copy link
Owner Author

leebyron commented Apr 2, 2019

Thanks for talking through it with me!

React actually has built in support for not rendering null or false as a string because of this exact pattern, so you wouldn't even see 'false' render.

I think understanding that boolean and && doesn't result in a true or false value but instead evaluates to the first non-falsey value is a weird Javascriptism for new comers to the language, but I agree that it's terse enough that it's worth learning. Generally I like ternary more than most, though I think ternary intermixed in JSX is often ugly.

I do think that simple uses of <If> are syntactically clearer, but I certainly agree that complex blocks of if/else quickly get less clear. The relative value add is just far less than <For>, which is why I'm thinking of ditching them from this lib

I think `<If>` and `<Else>` might be a distraction from the value of `<For>` and I'm considering remove it. They're also potentially helpful for clearer syntax compared to `{condition && <Node />}` but have some real drawbacks as well.

Perhaps we should consider including them in a `react-conditionals` library?
@superhawk610
Copy link

Well said. I think the relatively small value vs <For> is definitely the strongest reason for the change, focus is important.

@leebyron leebyron merged commit f15d039 into master Apr 2, 2019
@leebyron
Copy link
Owner Author

leebyron commented Apr 2, 2019

Done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants