Cleaner way to write ReactJS component

Question 1

My code is as follows :

import React, {PropTypes} from 'react';
import ReactCSSTransitionGroup from 'react-addons-css-transition-group';
import { filter_names } from './filterActions';
import Price from './price';
import Year from './year';
import Make from './make';
import Model from './model';
import Mileage from './mileage';
import Transmission from './transmission';
import Fuel from './fuel';

class filters extends React.Component {

    showFilters(filters){

        return filters.map(filter => {
            switch (filter.name){
                case filter_names.priceRange:
                    return <Price recap={filters.some(f => f.name === filter_names.priceRange)} title="Price" filters={filters} {...this.props}/>;
                case filter_names.year:
                    return <Year recap={filters.some(f => f.name === filter_names.year)} title="Year" filters={filters} {...this.props}/>;
                case filter_names.mileage:
                    return <Mileage recap={filters.some(f => f.name === filter_names.mileage)} title="Mileage" filters={filters} {...this.props}/>;
                case filter_names.fuel:
                    return <Fuel recap={filters.some(f => f.name === filter_names.fuel)} title="Fuel" filters={filters} {...this.props}/>;
                case filter_names.transmission:
                    return <Transmission recap={filters.some(f => f.name === filter_names.transmission)} title="Transmission" filters={filters} {...this.props}/>;
                default:
                    return true;
            }
        })
    }


    render(){
        const language = this.props.language;
        const filters = this.props.filters;

        return (
            <div className="filters noPadding col-xl-8 col-lg-6 col-md-6 col-sm-5 col-xs-12">
                <ReactCSSTransitionGroup transitionName="example" transitionAppear={true} transitionAppearTimeout={500} transitionEnterTimeout={500} transitionLeaveTimeout={500}>
                    {this.showFilters(filters)}
                </ReactCSSTransitionGroup>
            </div>
        );
    }

}

export default filters;

Is there any way to write it on a cleaner way? I have in showFilter function all those components. Do I need to repeat it for every component or is there any simpler way to do it?

Question 2

You haven’t shown any of the child components, but here’s how I would approach this:

Instead of having each of your components be generated in a giant switch statement, I’d use an object that maps a component name to a component itself. Each of these components all have the same prop keys, they are just different components, right? *

Further, your recap value is completely pointless. If you’re on that switch branch you don’t need to do a some on the filter array to check if at least one element exists in the array with that name – you already know it does, because you’re on that switch branch. As a result, I can simplify your showFilters method to:

const filterComponentMap = {
  priceRange: (props) => <Price title='Price' {...this.props} />
}

showFilters(filters, originalProps) {
  return filters.map((filter) => {
    return filterComponentMap[filter.name]({ filters, ...originalProps })
  }
}

You don’t include the declaration of filter_names, but if you wanted to use the keys of those rather than raw strings you could always do:

const filterComponentMap = {
  [filter_actions.priceRange]: (props) => ..
}

Note that by convention we name variables camelCase and not snake_case, so rename that filter_actions to filterActions. Classes should be named PascalCase, and component should always follow that convention (even though components aren’t always classes), so filters should be Filters (which is not a particularly good description either, by the way).

As your component is stateless there’s no need to have the boilerplate of extending it from React.Component, you can instead use a simple function:

function showFilters(filters, originalProps) {
  ...
}

const Filters = ({ language, filters }) => {
  return <div className="filters noPadding col-xl-8 col-lg-6 col-md-6 col-sm-5 col-xs-12">
    <ReactCSSTransitionGroup transitionName="example" transitionAppear={true} transitionAppearTimeout={500} transitionEnterTimeout={500} transitionLeaveTimeout={500}>
      {showFilters(filters, this.props)}
    </ReactCSSTransitionGroup>
  </div>
}

export default Filters

Consider using SASS (or some other preprocessor) and name your classes based on the component itself rather than the properties of the class. That is, instead of adding .col-xs-8 .col-md-6 to your component declaration, add a single class to it (named .filters or something, using BEM or whatever you want to use – I prefer CSS modules) and style it in SASS with the Bootstrap mixins like so:

.filters {
  @extend make-col($grid-breakpoints.md, 6);
  @extend make-col($grid-breakpoints.xs, 8);
}

And store this file alongside the component file named <ComponentName>.sass.

* note that this could be further simplified by doing something like this instead (for some values of simplified)

const filterComponentMap = {
  priceRange: Price
}

showFilters(filters, originalProps) {
  return filters.map((filter) => {
    return <{filtersComponentMap[filter.name]} filters={filters} {...originalProps} />
  }
}

But this feels almost generic to a fault and breaks down as soon as you want to add some unique props (boolean values, for instance) to any of your components.

Leave a Reply Cancel reply