Skip to content

Commit

Permalink
clarify existance checking
Browse files Browse the repository at this point in the history
  • Loading branch information
chantastic committed Oct 18, 2016
1 parent 400218a commit 996008f
Showing 1 changed file with 27 additions and 17 deletions.
44 changes: 27 additions & 17 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -411,32 +411,42 @@ See: [Compound State](#compound-state) pattern
## Existence Checking
Do not check existence of `prop` objects. Use `defaultProps`.
Do not check existence of props at the root of a component.
Componenst should not have two possible return types.

This comment has been minimized.

Copy link
@evandavis

evandavis Oct 18, 2016

typo: Componenst -> Components

This comment has been minimized.

Copy link
@chantastic

chantastic Oct 18, 2016

Author Owner

Thanks.

```javascript
// bad
render () {
if (this.props.person) {
return <div>{this.props.person.firstName}</div>;
} else {
return null;
}
const Person = props => {
if (this.props.firstName)
return <div>{this.props.firstName}</div>
else
return null
}
```
Componenents should *always* render. Consider adding `defaultProps`, where a sensible default is appropriate.

This comment has been minimized.

Copy link
@evandavis

evandavis Oct 18, 2016

Oops, one more typo. Components is tricky to type.

This comment has been minimized.

Copy link
@chantastic

chantastic Oct 18, 2016

Author Owner

Holy crap. Yah, it's like 50/50 on my getting that correct the first time.

```javascript
// good
class MyComponent extends React.Component {
render() {
return <div>{this.props.person.firstName}</div>;
}
// better
const Person = props =>
<div>{this.props.firstName}</div>

Person.defaultProps = {
firstName: "Guest"
}
```
MyComponent.defaultProps = {
person: {
firstName: 'Guest'
}
};
If a component should be conditionally rendered, handle that in the owner component.
```javascript
// best
const TheOwnerComponent = props =>
<div>
{props.person
? <Person {...props.person} />
: null

This comment has been minimized.

Copy link
@evandavis

evandavis Oct 18, 2016

Is the ternary preferred to {props.person && <Person {...props.person} />}?

This comment has been minimized.

Copy link
@chantastic

chantastic Oct 18, 2016

Author Owner

Yah, that'd definitely be preferred. Thanks.

}
</div>
```
This is only where objects or arrays are used. Use PropTypes.shape to clarify
Expand Down

0 comments on commit 996008f

Please sign in to comment.