React antipatterns in our project.md

1. Asynchronous setState. Here's the problem:

// Various async stuff should reside here
componentDidMount() {
    this._getCity();
    this._fetchCitiesAsync();
    this._timer = setInterval(() => this._fetchCitiesAsync(), 5000);
}

_getCity() {
    this.setState({city: localStorage.getItem("user_city") || localStorage.getItem("city") || "Москва"});
    this._selectedCityChanged; // But setState is actually asynchronous and changes can be batched before applied!
}

_selectedCityChanged() {
    localStorage.setItem("user_city", this.state.city);
    this._updateRateMenuLinks();
}

... and here's the solution:

// Verious async stuff should reside here
componentDidMount() {
    this._getCity();
    this._fetchCitiesAsync();
    this._timer = setInterval(() => this._fetchCitiesAsync(), 5000);
}

// In setState(updater, [callback]), updater can be both an object (like in our case), but also the (prevState, props) => stateChange function
_getCity() {
    this.setState(
        {city: localStorage.getItem("user_city") || localStorage.getItem("city") || "Москва"},
        this._selectedCityChanged
    );
}

_selectedCityChanged() {
    localStorage.setItem("user_city", this.state.city);
    this._updateRateMenuLinks();
}

2. Avoid recreating functions on each render. The problem: we need to access each item in the map from the onClick handler. However, in both our current versions of onClick, we are re-creating it each time we re-render:

_getCities() {
     const _cities = this.state.cities;

     return _cities.map(item => {
         return (
             <h3>
                <a key={item.id}
                   href="#breadcrumb"
                   className="contacts-choose-button"
                   onClick={() => this._handleClick(item)}> 
                   // OR: onClick={this._handleClick.bind(this, item)}
                   {item.name}
                 </a>
             </h3>
         );
     });
 }

The solution is to save the _onItemClick into a constant and pass it down as a prop to the child component (FAQListItem) along with the item. Notice that FAQListItem also saves _handleOnClick into a constant to prevent it from being re-created each render:

class FAQPage extends Component {
    constructor() {
        super();

        this.state = {...};

        this._onItemClick = this._onItemClick.bind(this);
    }
}

<FAQList
    topics={this.state.topics}
    onItemClick={this._onItemClick}
/>

const FAQList = ({ topics, onItemClick }) => { // Note how we destructure the props here
    return <div>
          {topics.map(item => {
                    return (
                        <div key={item.id}>
                            <a href={"#collapse" + item.id}>{item.name}</a>
                            <div id={"collapse" + item.id}>
                                <ul>
                                    { /* Note the lack of parentheses around "item" and the implicit return  */ }
                                    {item.qas.map(item =>
                                        /* {...{item, onItemClick}} is equivalent to {item: item, onItemClick: onItemClick}
                                        or item={item} onItemClick={onItemClick} */
                                        <FAQListItem key={item.id} {...{item, onItemClick}} />
                                    )}
                                </ul>
                            </div>
                        </div>
                    )
                }
            )}
    </div>
};

const FAQListItem = ({ item, onItemClick }) => { // Note how we destructure the props here
    // Note how we only create the function once and then pass a constant reference to it
    const _handleOnClick = () => onItemClick(item);

    return <li>
        <a href="#breadcrumb"
           onClick={_handleOnClick}>{item.question} </a>
        <div href={"#collapsefaq" + item.id}>
            <p>{item.answer}</p>
        </div>
    </li>
};

3. Render correctly. There is no need in declaring variables that will be conditionally initialized and in helpers that simply return pieces of JSX markup:

_getCities() {
    const _cities = this.state.cities;

    return _cities.map(item => {
        return (
            <h3>
                <a key={item.id}
                   href="#breadcrumb"
                   className="contacts-choose-button"
                   onClick={() => this._handleClick(item)}>
                   {item.name}
                </a>
            </h3>
        );
    });
}

render() {
    let city, phone, email, office, adress, skype, contact;
    const cities = this._getCities();

    if (this.state.currentCity) {
        if (this.state.currentCity.name.length > 0) {
            city = (<h3>{this.state.currentCity.name}</h3>);
        }
    }
    return (
        <div className="row" id="info">
            <div className="visible-xs">
                <div className="col-xs-12">
                    {city}
                    {cities}
                    <h3>Хотите, чтобы Gett был и в вашем городе?</h3>
                    <p>Свяжитесь с нами и мы все обсудим.</p>
                </div>
            </div>
        </div>
    )
}

Instead, we can go with conditional rendering using JS's schortcut evaluations for bollean expressions:

render() {
    const { cities } = this.state;

    return (
        <div className="row" id="info">
            <div className="visible-xs">
                <div className="col-xs-12">
                    { /* Note, however, that not every falsy value would work for this.state.currentCity 
                    -- e.g., it can't be an empty string or an empty array, only false or null */ }
                    {this.state.currentCity && this.state.currentCity.name.length > 0 && <h3>{this.state.currentCity.name}</h3>}
                    {cities.map(item => (
                        <h3>
                            <a key={item.id}
                               href="#breadcrumb"
                               className="contacts-choose-button"
                               onClick={() => this._handleClick(item)}> { /* This is an issue as we know by now  */ }
                               {item.name}
                            </a>
                        </h3>
                    ))}
                    <h3>Хотите, чтобы Gett был и в вашем городе?</h3>
                    <p>Свяжитесь с нами и мы все обсудим.</p>
                </div>
            </div>
        </div>
    )
}