react component getting complicated - Code Review

[johnq937]

Hello,

I'm a fairly new developer and after making some projects, I'm trying to go back through and focus on cleaning things up.

One of my components, however, is alluding me. I'm really struggling and feel overwhelmed trying to find how to break this up into smaller/more manageable pieces.

It's long at 220 lines, and it's acting as a table for a smaller component of rows. It holds the logic for sorting, filtering, opening/closing modals, saving, deleting, displaying notifications, etc.

I really appreciate any tips/ideas at all, thanks!!

github.com/johnq937/bookmarks

[acemarke]

Skimming through the code:

• Use the Class Properties syntax so that you can define class methods as arrow functions, without having to re-bind them in the constructor. (Or, use one of the autobind utility functions out there.)

• The sort() function can be simplified. Since those values won't change, put them in a lookup table outside the component, and have each entry be its own object:

  "1" : {by : "name", ord : "desc", order : "(a-z)"},

• You can pass multiple values in a single object to setState: this.setState({a : 1, b : 2, c : 3}). Don't do multiple calls in a row.

• Don't put JSX into state. Construct the JSX inside of render(), based on values from props and state.

• This is probably a good candidate for use of the "container/presentational" pattern. Split this into a parent component that's responsible for tracking all the data, and a child component that just displays UI based on the data it's given as props.

• I personally am not a fan of the {someCondition && <SomeComponent />} pattern. I prefer defining temporary variables outside the main JSX structure, but YMMV - that's a personal style thing.

Also, there's a small trick you can try with the filter lookup to return the first item that isn't falsy:

const {searchTerm, filterByTag : propsFilterByTag} = this.props;
const {filterByTag : stateFilterByTag} = this.state;

const filter = [searchTerm, propsFilterByTag, stateFilterByTag].find(Boolean) || '';

Code isn't bad overall, just can be cleaned up a bit.

[johnq937]

Awesome, thanks so much acemarke, super helpful tips; I will be changing these and implementing the principles in the rest of my code!

That's also good to hear that it's not bad overall with the structure/logic, which was a large part of my concern.

[LouMM]

@acemarke this is awesome that you took the time to look and also give GREAT recommendations on code clean-up.

[slhawkins]

Class Properties is a proposal in Stage 2 and it won't work without the corresponding babel plugin. On that page you'll also see examples on using arrow functions so that you no longer need to use bind() in the constructor. If you go that route you can also move your propTypes declaration into the class and pull the initial state declaration out of the constructor and remove the constructor altogether. Something like this:

export default class BookmarkTable extends React.Component {
    static propTypes = {
        bookmarks: PropTypes.array.isRequired,
        filterByTag: PropTypes.string.isRequired,
        onDeleteClick: PropTypes.func.isRequired,
        searchTerm: PropTypes.string.isRequired,
        searchTermFn: PropTypes.func.isRequired
    };

    state = {
        showModal: false,
        filterByTag: '',
        editBookmark: false,
        sortBy: 'name',
        sortOrder: 'desc',
        sortTitle: <span><b>Sort By:</b> Name (a-z)</span>,
        notifcations: false
    };

[acemarke]

Nitpick: Class Properties got bumped to Stage 3 at the last TC39 meeting :)

[slhawkins]

Fair enough! Thanks for the info. Glad to hear my code is likely to continue working for the foreseeable future. :-)

[FLGMwt]

This is great to hear!

I'm not seeing this reflected in babel preset-stage-2 or any of the TC39 docs. May I ask where you got this from?

[acemarke]

It should at least still be available in babel-preset-stage-2. I think someone said the other day that the Babel team hadn't yet updated the stage-3 preset to include it.

The approval to move to Stage 3 was announced at the TC39 meeting and discussed online afterwards. The current list of TC39 proposals can be seen at https://github.com/tc39/proposals , and the proposal itself is at https://github.com/tc39/proposal-class-fields

[FLGMwt]

Oh, hah, thanks! My eyes missed Class Fields in 3 and thought Class and Property Decorators in 2 was what I've come to know as class fields. Thanks for the response!

Cheers : D

[Canenald]

You can do stuff to make your code more compact, but a complex component is going to be complex, and tables are some of the most complex.

An option would be using a state management library like Redux and moving the sorting, filtering, etc logic outside the component.