Multiple redux connections vs deeply nested props performance/best practices


(haxxxton) #1

I have a fairly complex app that leverages React-Redux, ImmutableJS, and Redux-Immutable among things. I am starting to get to the point that im beginning to notice performance issues with my current build. Unfortunately, I am not in a development scenario where i have fellow React developers to use as sounding boards for ideas, so my direction has really been based off much smaller app architecture designs. As such, I wanted to see if the decisions i have made surrounding these issues make sense, or if there are glaringly obvious changes that I should make.

Store Design
I dont think there is really a better way to organise the store. It is a ‘flat’ as possible given the data at hand. It basically takes the form of:

Store: Map({
	Foo: Map({
		Items: Map({
			1: Map({
				id: 1,
				title: 'Item 1',
				languages: List([2, 3]),
			}),
			2: Map({
				id: 2,
				title: 'Item 2',
				languages: List([1, 3]),
			}),
			...
		}),
		State: Map({
			isDirty: false,
			isRequesting: false,
		})
	}),
	Languages: Map({
		1: 'Python',
		2: 'Ruby',
		3: 'Javascript',
		...
	}),
	...
})

Each number keyed Map (Foo.Items, Languages, etc.) is mostly populated as a result of an API call followed by using ImmutableJS’s fromJS on the response.

Please excuse the contrived examples below, there is a separate HOC being used (but not displayed) that converts the Immutable Items returned in the mapStateToProps call, into regular Javascript Array or objects.

Option A: Fewer connect more complex props
I am not sure where the inkling came from, or if it was merely bred from laziness of importing and wrapping with the connect HOC, but at some point I got it into my head that it was more performant/better practice to make fewer calls to connect and then pass the resultant complex items down to child components. For example:

import PropTypes from 'prop-types';
import React from 'react';
import { connect } from 'react-redux';

const Bar = ({ item }) => {
	return (
		<div>
			<p>{ item.title }</p>
			<p>{ item.languages.join(', ') }</p>
		</div>
	);
};

Bar.propTypes = {
	item: PropTypes.shape({
		id: PropTypes.number,
		title: PropTypes.string,
		languages: PropTypes.arrayOf(PropTypes.number),
	}),
};

const BarList = ({ items }) => {
	return (
		<div>
			<p>List of items</p>
			{
				items.map(item => (
					<Bar
						key={ item.id }
						item={ item }
					/>
				))
			}
		</div>
	);
};

BarList.propTypes = {
	items: PropTypes.arrayOf(
		PropTypes.shape({
			id: PropTypes.number,
			title: PropTypes.string,
			languages: PropTypes.arrayOf(PropTypes.number),
		}),
	),
};

const mapStateToProps = (state) => {
	const items = state.getIn(['Foo', 'Items']).toList();
	return {
		items,
	};
};

export default connect(mapStateToProps)(BarList);

In this example, Bar is receiving the entire item prop as an object. (I understand i could alleviate some of the performance hits by using { ...item }, in my app there are potentially conflicting props if using the spread operator)

Option B: More connect less complex props
This is where I am thinking I should be heading towards. My hunch is telling me that the comparison of immutable objects for difference is more performant when React is making the shouldComponentUpdate call behind the scenes. This also means that identifying what exactly has changed should be easier to debug, as only flat props are ever being passed into child components

import PropTypes from 'prop-types';
import React from 'react';
import { connect } from 'react-redux';

const BarSmarter = ({ item }) => {
	return (
		<div>
			<p>{ item.title }</p>
			<p>{ item.languages.join(', ') }</p>
		</div>
	);
};

BarSmarter.propTypes = {
	item: PropTypes.shape({
		id: PropTypes.number,
		title: PropTypes.string,
		languages: PropTypes.arrayOf(PropTypes.number),
	}),
};

const mapStateToPropsForBar = (state, ownProps) => {
	const item = state.getIn(['Foo', 'Items', ownProps.itemId]);
	return {
		item,
	};
};

const Bar = connect(mapStateToPropsForBar)(BarSmarter);

const BarList = ({ itemIds }) => {
	return (
		<div>
			<p>List of items</p>
			{
				itemIds.map(id => (
					<Bar
						key={ id }
						itemId={ id }
					/>
				))
			}
		</div>
	);
};

BarList.propTypes = {
	itemIds: PropTypes.arrayOf(PropTypes.number),
};

const mapStateToPropsForBarList = (state) => {
	const itemIds = state.getIn(['Foo', 'Items']).keys();
	return {
		itemIds,
	};
};

export default connect(mapStateToPropsForBarList)(BarList);

In this example, we connect both components, and just pass an id to the child (Bar), which fetches its own data from the store.


So essentially what I am trying to determine is… which is better practice? which is more performant? is there an Option C… D… E for how to make this pattern as performant as possible?


(Troy Rhinehart) #2

Presentational components should be simple, using only props/state and be written stateless when possible. Container components which provide the context to Presentational components can have nested Container components, however it should be avoided when possible.

Your Presentational components can leverage React.PureComponent or your own variant of a shouldComponentUpdate to help with wasted renders. Just keep in mind Immutable objects are shallow compared so if you’re comparing state from a store there should be no issues, but if you reduce/map/filter the state in your mapStateToProps logic you risk creating a new Immutable object and void default shallow compare logic.

Your option A is the ideal solution IMO, this minimizes the amount of listeners and times the HOC of connect has to be called to pull state and map via your mapStateToProps calls.

You mentioned something interesting

there is a separate HOC being used (but not displayed) that converts the Immutable Items returned in the mapStateToProps call, into regular Javascript Array or objects

This IMO is bad, keep your props as Immutable objects if they’re purely from store state, this way when you compare curProps.arr to nextProps.arr they’ll be the same if they were not modified. Then, in your render method use Immutable.get instead of toJS since that conversion comes at a cost.