1
\$\begingroup\$

I've coded a React component which should display search result and give the user the option to scroll down and retrieve 25 more objects to be displayed when the scrollbar hits the bottom. What I've noticed when I compile and test the component is that the filter buttons to mark is really slow: it takes a couple of seconds after I click on it till it gets marked, and the same applies for bookmark buttons. The component feels really sluggish when it loads in. However, for each time more items are loaded in, it also generates more publishedStates for the publishedButton. I notice that as when it gets generated over 1000 states and 1000 objects is loaded, when I click on buttons to change the state on filter etc is slow.

How can I improve and optimize the performance of my component?

import * as React from "react";
import * as _ from "lodash";
declare const $: any

interface Props {
    userId: number;
}

interface State {
    objects: Object | null
    isVisible: Boolean
    searchString: string
    bookmarkStates: Array<boolean>
    publishedStates: Array<boolean>
    categories: Array<String>
    languages: Array<String>
    published: Array<String>
    hasMoreObjects: Boolean,
    storiesLoaded: number
}

const getStateName = function (isFiltered, state) {
    if (JSON.stringify(isFiltered)==JSON.stringify(state.commodityStates))
        return 'commodityStates';
    else if (JSON.stringify(isFiltered)==JSON.stringify(state.languageStates))
        return 'languageStates';
    else
        return 'publishedStates';
}

const handlePublishedTime = function (publishedTime) {
    let publishedTimeCopy = publishedTime.split(' ')
    let monthNames = [ "January", "February", "March", "April", "May", "June", 
                        "July", "August", "September", "October", "November", "December" ];
    let todayDate = new Date();
    let dd = todayDate.getDate();
    let mm = todayDate.getMonth();
    let yyyy = todayDate.getFullYear();
    // Used for comparison of dates.
    let todayDateFormat = dd + ', ' +  monthNames[mm] + ' ' + yyyy;     
    let publishedTimeFormat =   publishedTimeCopy[2].split(',') + ' ' + publishedTimeCopy[3] + ' ' + publishedTimeCopy[4]; 

    if (todayDateFormat == publishedTimeFormat) {
        return publishedTimeCopy[0];
    }

    return publishedTimeCopy[2].replace(',',' ') + ' ' + Resources[publishedTimeCopy[3]].substring(0,3)
}

export default class SearchResult extends React.PureComponent<Props, State> {  
    constructor(props) {
        super(props);

        this.state = {
            Object: null,
            isVisible: true,
            searchString: "",
            bookmarkStates: [],
            commodityStates: [],
            languageStates: [],
            publishedStates: [],
            categories: [],
            languages: [],
            published: ['1D', '2D', '1W', '1Y', '5Y'],
            hasMoreObjects: true,
            objectsLoaded: 25
        }

        $.get('/webapi/test, function (data) {
            this.setState({
                Object: data
            });
        }.bind(this));
    }

    handleScroll = (e) => {
        const bottom = e.target.scrollHeight - e.target.scrollTop === e.target.clientHeight;
        if (bottom) {  
            this.setState({
                storiesLoaded: this.state.storiesLoaded + 25
            });
        }
    }

    handleSearchTextInput (event) {
        this.setState({
            searchString: event
        });
    }

    handleSearchButton () {
        this.setState({
            isVisible: true
        });
    }

    addNewBookmark = function (userId, newsId, isBookmarked, bookmarkIndex) {
        $.post('/webapi/newstestAddNewBookmark?userId=' + (userId), 
        { 
            UserID: userId,
        })
        .done(function () {
            this.changeBookmarkState(isBookmarked, bookmarkIndex);
        }.bind(this))   
    }

    removeBookmark = function (userId, newsId, isBookmarked, bookmarkIndex) {
        $.post('/webapi/test/RemoveBookmark?userId=' + (userId), {})
        .done(function () {
            this.changeBookmarkState(isBookmarked, bookmarkIndex);
        }.bind(this))
    }

    changeBookmarkState(isBookmarked, index) {
        let bookmarkStatesCopy = this.state.bookmarkStates.slice(); // Create a copy to avoid mutation.
        bookmarkStatesCopy[index] = isBookmarked == true ? false : true;

        this.setState({
            bookmarkStates: bookmarkStatesCopy
        });
    }

    changeFilterState(filterByState, index) {
        const stateName = getStateName(filterByState, this.state);
        let filterByStateCopy = filterByState.slice(); // Create a copy to avoid mutation.

        if (index == 0) {
            filterByStateCopy[index] = filterByState[index] === false;
            if (stateName != 'publishedStates')
                filterByStateCopy.map(({}, index1) => {
                    filterByStateCopy[index1+1] = false;
                });
        }
        else {
            filterByStateCopy[index] = filterByState[index] === false;
            stateName != 'publishedStates' ? filterByStateCopy[0] = false : null
        };

        this.setState({
            [stateName]: filterByStateCopy
        } as any);
    }

    renderBookmarkButton(userId, newsId, isBookmarked, bookmarkIndex) {  
        return (
            <a onClick={()=>{isBookmarked == false ? this.addNewBookmark(userId, newsId, isBookmarked, bookmarkIndex) : this.removeBookmark(userId, newsId, isBookmarked, bookmarkIndex);}}>
                <img className={isBookmarked == false ? "bookmark-button" : "bookmark-button bookmark-button-fill-color"} src="/images/logo/Bookmark.svg" />
            </a>
        );
    }

    renderCategories(Object , isFiltered) {   
        let categories = this.state.categories;

        Object.map((object) => {
            if (Object == null)
                return null;

            object.Categories.map((categoryObj, index) => {
                if (categories[index] == null)
                    categories.push(categoryObj)
                    if(index === 0)
                        categories.splice(0, 0, 'All');         
            });
        });

        // Used to create a new array which contains all elements that pass the test.
        categories = categories.filter((value, index, array) => 
             !array.filter((v, i) => JSON.stringify(value) == JSON.stringify(v) && i < index).length);

        const mappedCategories = categories.map((category, index) => {
            if (isFiltered[index] == null) {
                isFiltered[index] = false; 
                if (isFiltered[0] == false) {
                    isFiltered[0] = true;    
                }
            };

            return (
                <div key={index}>
                    <a className="checkbox-display" onClick={()=>{ this.changeFilterState(isFiltered, index);}}>
                    {isFiltered[index] ? 
                        <div className={ isFiltered[index] ? "checkbox-rectangle checkbox-rectangle-fill-color horizontal-row-filterby" : "checkbox-rectangle horizontal-row-filterby"}>
                            <img className="checkbox-image-size" src="/images/logo/checkmark.svg"/>
                        </div> : 
                        <div className="checkbox-rectangle horizontal-row-filterby"/>
                    }
                    </a>
                        <span className="category1">{index == 0 ? 'All' : category['Name']}</span>              
                </div>        
            );
        });

        return (
            <div>
                <div className="filter-by-commodity">{Resources.Commodity}</div>
                {mappedCategories}
           </div>
        );       
    }

    renderLanguage(Object , languageFilter) {   
        let languages = this.state.languages;

        Object.map((object ) => {
            if (Object == null)
                return null;

            object.Languages.map((languageObj, index) => {         
                if (languages[index] == null){
                    languages.push(languageObj)
                    if(index === 0){
                        languages.splice(0, 0, 'All');
                    };
                };
            });
        });

        // Used to create a new array which contains all elements that pass the test.
        languages = languages.filter((value, index, array) => 
             !array.filter((v, i) => JSON.stringify(value) == JSON.stringify(v) && i < index).length);

        const mappedLanguages = languages.map((language, index) => {
            if (languageFilter[index] == null) {
                languageFilter[index] = false; 
                if (languageFilter[0] == false) {
                    languageFilter[0] = true;
                }
            };

            return (
                <div key={index}>
                    <a onClick={()=>{ this.changeFilterState(languageFilter, index);}}>
                    {languageFilter[index] ? 
                        (<div className={languageFilter[index] ? "checkbox-rectangle checkbox-rectangle-fill-color horizontal-row-filterby" : "checkbox-rectangle horizontal-row-filterby"}>
                            <img className="checkbox-image-size" src="/images/logo/checkmark.svg"/>
                        </div>) : <div className="checkbox-rectangle horizontal-row-filterby"/>
                    }
                    </a>
                        <span className="category1">{index == 0 ? 'All' : language['LanguageName']}</span>              
                </div>        
            );
        });

        return (
            <div>
                <div className="horizonal-line-short" />
                <div className="filter-by-commodity">{Resources.Languages}</div>
                {mappedLanguages}
            </div>
        );
    }

    renderPublished(publishedFilter) {
        let published = this.state.published;

        const mappedPublished =
            published.map((pushlish , index) => {
                if (publishedFilter[index] == null) 
                        publishedFilter[index] = (false); 

                return (
                    <div key={index}>
                        <a onClick={()=>{ this.changeFilterState(publishedFilter, index);}}>
                                <div className={publishedFilter[index] ? "checkbox-rectangle-large vertical-row-filterby checkbox-rectangle-fill-color" : "checkbox-rectangle-large vertical-row-filterby"}>
                                    <span>{pushlish}</span>    
                                </div>
                        </a>          
                    </div>        
                );
            });

        return (
            <div>
                <div className="horizonal-line-short" />
                <div className="filter-by-commodity">{Resources.Published}</div>     
                <div className='checkbox-display'> {mappedPublished}</div>     
            </div>
        );
    }

    renderStories(Object ) {
        let isBookmarked = this.state.bookmarkStates;

        if(Object == null)
            return null;

        const mappedStories = 
            Object.slice(0, this.state.objectsLoaded).map((object , index) =>  {
                if (isBookmarked[index] == null) 
                    isBookmarked[index] = object.IsBookmarkedMain;

                return (
                    <div key={index} className={index == 0 ? "object-rectangle-no-bg" : index % 2 == 0 ? 'object-rectangle-no-bg' : 'object-rectangle'}>
                        <span className="published-time">{handlePublishedTime(object.PublishedTime)}  </span>
                        <div>
                            <a className="object-header" href={"/test/object.aspx?newsId=" + object.ID}>{object.Header}</a>
                            {this.renderBookmarkButton(this.props.userId, object.Id, isBookmarked[index], index)}
                        </div>
                    </div>
                );   
            });

        return (
            <div>
                {mappedStories}
            </div>
        );
    }

    renderResults(Object, isVisible, commodityStates, languageStates, publishedStates) {
        if (isVisible){
            return (
                <div className="search-result-master-container">
                    <span className="section-header">{Resources.Filter_By}</span>
                    <span className="section-header">{Object.length} {Resources.Results}:</span>
                    <div id="filterBySideBar">
                        <div className="horizontal-line-filter">
                            <div>{this.renderCategories(Object , commodityStates)}</div>
                            <div>{this.renderLanguage(Object , languageStates)}</div>
                            <div>{this.renderPublished(publishedStates)}</div>
                        </div>
                    </div>
                    <div className="horizontal-line-stories-top"/>
                    <div className="scrollbar"  onScroll={this.handleScroll.bind(this)}>
                        <div className="force-overflow">
                                {this.renderStories(Object)}
                        </div>
                    </div>
                    <div className="horizontal-line-stories-bottom" />
                </div>    
            );       
        }                 
    }

    render() {
        let Object = this.state.Object;
        let isVisbible = this.state.isVisible;
        let commodityStates = this.state.commodityStates;
        let languageStates = this.state.languageStates;
        let publishedStates = this.state.publishedStates;

        if (Object == null)
            return null;

        let renderShowResults = this.renderResults(Object , isVisbible, commodityStates, languageStates, publishedStates);

        return renderShowResults
    };
} 
\$\endgroup\$
1
\$\begingroup\$

rerenders from bind and anonymous functions

You have a really bad rerendering problem. Every click handler you have is either binded or an anonymous function. Turn these into named functions

<div className="scrollbar"  onScroll={this.handleScroll.bind(this)}>

bind creates a new instance of the function during render cycles, which will add up when you have 10 of them with each instance of the function.

Instead, make it an arrow function inside your class. Arrow functions do not need bind

handleScroll = () => {}
<div className="scrollbar"  onScroll={this.handleScroll}>

anonymous functions have the exact same problem in event listeners/handlers

<a className="checkbox-display" onClick={()=>{ this.changeFilterState(isFiltered, index);}}>

But a function that takes in arguments are immediately invoked, so you can do an arrow function as a double return.

changeFilterState = (isFiltered, index) => () => {} // two arrow functions. this function returns another function

<a className="checkbox-display" onClick={this.changeFilterState(isFiltered, index)}>

separate your UI elements into smaller components

When you have a large component, its render cycle will go through every single ui element in the component. But not all elements need to be rerendered. Elements whose values never change (or almost never) don't need to go through another render cycle. In order to accomplish this, you need to split them up into their own components.

memoize those smaller components with React.memo for easy wins.

React.memo makes an initial check to see if anything has changed between the the current and next set of data inside the component. If React.memo finds them to be equal, the component doesn't go through a render cycle.

A method that returns many ui elements

<div key={index}>
    <a className="checkbox-display" onClick={()=>{ this.changeFilterState(isFiltered, index);}}>
    {isFiltered[index] ? 
        <div className={ isFiltered[index] ? "checkbox-rectangle checkbox-rectangle-fill-color horizontal-row-filterby" : "checkbox-rectangle horizontal-row-filterby"}>
            <img className="checkbox-image-size" src="/images/logo/checkmark.svg"/>
        </div> 
        : 
        <div className="checkbox-rectangle horizontal-row-filterby"/>
    }
    </a>
        <span className="category1">{index == 0 ? 'All' : category['Name']}</span>              
</div>

How often should the image change? More likely than not, those images aren't going to change for every single one of them each time you load more images. You can pull out that UI element and React.memo it so it does an initial check and skips the render process completely if the values are the same.

 const CategoryImage = (props) => {
  const { isFiltered } = props
  return (
    <React.Fragment>
      {isFiltered ? 
          <div className={ isFiltered ? "checkbox-rectangle checkbox-rectangle-fill-color horizontal-row-filterby" : "checkbox-rectangle horizontal-row-filterby"}>
              <img className="checkbox-image-size" src="/images/logo/checkmark.svg"/>
          </div> 
          : 
          <div className="checkbox-rectangle horizontal-row-filterby"/>
      }
    </React.Fragment>
  )

export default React.memo(CategoryImage)

and now it is this:

<div key={index}>
  <a className="checkbox-display" onClick={()=>{ this.changeFilterState(isFiltered, index);}}>
    <CategoryImage isFiltered={isFiltered[index]} />
  </a>
  <span className="category1">{index == 0 ? 'All' : category['Name']}</span>              
</div>

Smaller components are better for performance, because you get to pick and choose what should rerender and what should not. They are also much easier to reason.

Note that React.memo isn't the silver bullet to all render problems and you have to split up components intelligently. React.memo does a shallow check, which means it's very good with primitives, like strings, booleans and numbers. Notice that the parts I pulled out into its own separate component only has a boolean as a prop. That was very intentional. React.memo can easily do a comparison and stop a rerender.

Edit: Apologies, I didn't notice that this.handleScroll is already an arrow function. You don't need to bind in that case. Here, all I did was remove the bind inside the click handler.

handleScroll = (e) => {
  const bottom = e.target.scrollHeight - e.target.scrollTop === e.target.clientHeight;
  if (bottom) {  
      this.setState({
          storiesLoaded: this.state.storiesLoaded + 25
      });
  }
}

renderResults(Object, isVisible, commodityStates, languageStates, publishedStates) {
  if (isVisible){
      return (
          <div className="search-result-master-container">
              <span className="section-header">{Resources.Filter_By}</span>
              <span className="section-header">{Object.length} {Resources.Results}:</span>
              <div id="filterBySideBar">
                  <div className="horizontal-line-filter">
                      <div>{this.renderCategories(Object , commodityStates)}</div>
                      <div>{this.renderLanguage(Object , languageStates)}</div>
                      <div>{this.renderPublished(publishedStates)}</div>
                  </div>
              </div>
              <div className="horizontal-line-stories-top"/>
              <div className="scrollbar"  onScroll={this.handleScroll}>
                  <div className="force-overflow">
                          {this.renderStories(Object)}
                  </div>
              </div>
              <div className="horizontal-line-stories-bottom" />
          </div>    
      );       
  }                 
}

But for a function with a callback, you can write it out to have the functions be double returns:

addNewBookmark = (userId, newsId, isBookmarked, bookmarkIndex) => () => {
  $.post('/webapi/newstestAddNewBookmark?userId=' + (userId), 
  { 
      UserID: userId,
  })
  .done(() => {
    this.changeBookmarkState(isBookmarked, bookmarkIndex);
  }
}

removeBookmark = (userId, newsId, isBookmarked, bookmarkIndex) => () => {
  $.post('/webapi/test/RemoveBookmark?userId=' + (userId), {})
  .done(() => {
      this.changeBookmarkState(isBookmarked, bookmarkIndex);
  }
}

renderBookmarkButton(userId, newsId, isBookmarked, bookmarkIndex) {  
  return (
      <a onClick={isBookmarked == false ? this.addNewBookmark(userId, newsId, isBookmarked, bookmarkIndex) : this.removeBookmark(userId, newsId, isBookmarked, bookmarkIndex)}>
          <img className={isBookmarked == false ? "bookmark-button" : "bookmark-button bookmark-button-fill-color"} src="/images/logo/Bookmark.svg" />
      </a>
  );
}

Notice how I don't have () => inside the onClick

\$\endgroup\$
  • \$\begingroup\$ Hi, thanks for the answer. Could you provide me an example of the code with handleScroll = () => {} <div className="scrollbar" onScroll={this.handleScroll}>? I'm not sure how it should look like. \$\endgroup\$ – Fredrik Linger Jan 18 at 16:55
  • \$\begingroup\$ Apologies, handleScroll isn't the best example because you already have it as an arrow function. I left the arguments off and the rest of the function's content blank. But if you truly need a complete version, I'll do that now \$\endgroup\$ – Andrew Jan 18 at 20:55
  • \$\begingroup\$ That being said, memoizing is far more effective for your purposes \$\endgroup\$ – Andrew Jan 18 at 21:01
  • \$\begingroup\$ thanks a lot my friend! I understand the () => part now. What does it mean with callback? I'm really interested in learning the memo method, but how can I adapt my code to work with it? I'm also not sure how I can split up my component into smaller parts, do you have any example? :) I really appreciate your help, since I feel like I learn a lot more from seeing how you think and solve this, rather than reading bad examples. \$\endgroup\$ – Fredrik Linger Jan 18 at 21:43
  • \$\begingroup\$ When you say split up into smaller component, do you mean const in same file, or many several files? \$\endgroup\$ – Fredrik Linger Jan 18 at 21:53

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.