1
\$\begingroup\$

TableContainer.js does the common task of fetching data and passing it to a lower component, Table.js, which is a stateless functional component.

currentPage is stored in redux, I did this just to practice redux.

Question 1

Is this all reasonable?

Question 2

I noticed that the component will re-render when it receives a new currentPage and then re-renders again, once the data is loaded. The first re-render is not necessary as nothing changes before data is actually loaded. Should I just live with this or should I implement shouldComponentUpdate() to check if data has changed. Or is that potentially even more costly than the re-render itself?

Question 3

Is using componentDidUpdate() to check if currentPage has changed and then re-load the data a good way of controlling the load process?

Question 4

Is building the URL this way acceptable?

const pageParam = currentPage ? "?_page=" + currentPage : "";
fetch('https://jsonplaceholder.typicode.com/posts/' + pageParam)

TableContainer.js

import React from 'react';
import PropTypes from 'prop-types';
import Table from "../components/Table";
import Pagination from "../components/Pagination";
import {connect} from "react-redux";
import {changePage} from "../js/actions";


const PAGE_COUNT = 10;

const mapStateToProps = state => {
    return { currentPage: state.currentPage }
};

const mapDispatchToProps = dispatch => {
  return {
    changePage: page => dispatch(changePage(page))
  };
};

class ConnectedTableContainer extends React.Component {
    state = {
        data: [],
        loaded: false,
    };

    handlePageChange = page => {
        if (page < 1 || page > PAGE_COUNT) return;
        this.props.changePage(page);
    };

    loadData = () => {
        this.setState({ loaded: false });
        const { currentPage } = this.props;
        console.log("load data: " + currentPage);
        const pageParam = currentPage ? "?_page=" + currentPage : "";
        fetch('https://jsonplaceholder.typicode.com/posts/' + pageParam)
            .then(response => {
                if (response.status !== 200) {
                    console.log("Unexpected response: " + response.status);
                    return;
                }
                return response.json();
            })
            .then(data => this.setState({
                data: data,
                loaded: true,
            }))
    };

    componentDidMount() {
        this.loadData(this.props.currentPage);

    }

    componentDidUpdate(prevProps) {
       if (prevProps.currentPage != this.props.currentPage) {
            this.loadData();
        }
    }

    render() {
        const { loaded } = this.state;
        const { currentPage } = this.props;
        console.log("render page: " + currentPage);
        return (
            <div className="container">
                <div className="section">
                    <Pagination onPageChange={ this.handlePageChange } pageCount={ PAGE_COUNT } currentPage={ currentPage }/>
                </div>
                <div className={ "section " + (loaded ? "" : "loading") }>
                    <Table data={ this.state.data } />
                </div>
            </div>
        )
    }
}

ConnectedTableContainer.propTypes = {
    changePage: PropTypes.func.isRequired,
    currentPage: PropTypes.number.isRequired,
};

ConnectedTableContainer.defaultProps = {
    currentPage: 1,
};

const TableContainer = connect(mapStateToProps, mapDispatchToProps)(ConnectedTableContainer);

export default TableContainer;
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

Question 1

Is this all reasonable?

Absolutely!

Question 2

I noticed that the component will re-render when it receives a new currentPage and then re-renders again, once the data is loaded. The first re-render is not necessary as nothing changes before data is actually loaded. Should I just live with this or should I implement shouldComponentUpdate() to check if data has changed. Or is that potentially even more costly than the re-render itself?

This is normal, and I wouldn't worry about it. In fact, not having to worry about these types of things is a major goal of React.

It's important to realize that re-renders in React only update the DOM when there are changes and only update the relevant DOM nodes. DOM operations are very time consuming, because DOM objects are incredibly heavy objects, and updates have knock-on effects. This is one of the main reasons React doesn't use DOM objects internally. Instead, it uses a virtual DOM. Through a process called reconciliation, React can update the real DOM from the virtual DOM incredibly fast.

In the state where the page changes, but you don't have data yet, React will call render(), very quickly realize there are no changes to the element tree, then stop - this probably happens in roughly a millisecond.

On that note, you do need to be careful when you use Redux - specifically react-redux / connect(). Try to keep mappings of state to props as minimal as possible – don't map an entire object from the Redux tree to a prop for a component if you only need one property of that object. The reason this is important, is because:

  • Your reducers generate new objects every time they run (if you're using them correctly)
  • Redux and React use JavaScript's default comparison when deciding if it should update a component
  • Every object is distinct in JavaScript, even if their structure is identical

So a reducer takes one object, then generates a new object, and even if the properties of these objects are exactly the same, React/Redux will treat it like a change and update components. This is OK for one component, but if you make this a habit your application, you'll be missing out on one of React's biggest features: performance.

// Do this:

const GoodComponent = (props) => `page number: ${props.page}`;

connect(state => {
  page: pagination.page
})(GoodComponent)


// ... not this:

const BadComponent = (props) => `page number: ${props.pagination.page}`;

connect(state => {
  pagination: pagination
})(BadComponent)

Question 3

Is using componentDidUpdate() to check if currentPage has changed and then re-load the data a good way of controlling the load process?

Absolutely - this is probably why componentDidUpdate() was created.

Question 4

Is building the URL this way acceptable?

const pageParam = currentPage ? "?_page=" + currentPage : ""; fetch('https://jsonplaceholder.typicode.com/posts/' + pageParam)

It's probably OK for now, although you could use template literals. I generally avoid constructing the query string manually, except for the simplest cases. This is because the second you have multiple query params and some of them are optional, the construction gets tricky – what adds the question mark? how to handle trailing ampersands? trailing question mark?. Instead, use the URL API's searchParams property. Or, find a library that gives you more conveniences, like being able to provide query params as a dictionary/object in fetch()'s second argument.

\$\endgroup\$
1
  • \$\begingroup\$ Johntron, this was excellent, thank you so much. \$\endgroup\$
    – M3RS
    Nov 7, 2018 at 8:54

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.