Improve usage of immutable

[ca057]

Hello!

I came across this repo via the article you posted on Medium. First of all - thanks for it and for providing an easy repo with an already configured setup.

I found some minor improvements around your usage of immutable and would like to discuss them with you:

One of the main advantages of immutable when used with React is to be able to make easy and cheap checks (prevProps.myListOfUsers === nextProps.myListOfUsers) to understand if we need to perform an expensive update (re-rendering) of a component. In addition, as there can be no sneaky mutation somewhere in our code, finding bugs and dealing with data becomes much easier.

As we have to use an external library for that, we should try to stay in the "ecosystem" of the library as much as possible and follow their best practices. In the case of immutable, we have a rich API which provides us possibilities to update the data and leave the decision on how to do this efficiently to immutable itself (the library itself knows best how to update its internal structures).

The example you showed breaks this pattern, which can lead to performance issues in a larger application or in cases where a more complex state is involved. In the simple example you showed, my remarks might seem a bit artificial, but I think it is good to follow the best practices from the beginning 😊 Let me jump into the code:

In src/Counter/reducer.js you show the following update functions:

const increment = state =>
  state.merge({
    count: ++state.toJS().count
  });

const decrement = state =>
  state.merge({
    count: --state.toJS().count
  });

The main problem I see here, is that you call toJS inside your reducer. In case of this small state, this operation is simple and easy. Imagine that the state is growing, has multiple large lists or maps etc - then, this operation can get expensive. If all reducers implement this pattern and multiple of them are called often, this can hurt your performance. In addition, immutable is implemented in a way that it tries to reuse existing data which has not changed. If you are not careful, a pattern like this might break this internal data sharing.

Therefore, it is recommended to deal as long as you can with the data structures immutable provides you with and convert them to standard JS data structures as late as possible (e.g. right before passing them down). Parts of the API of immutable behaves similar to standard JS data structures, so you have no difference if you do something like new Immutable.List([]).map() or new Array().map(). In addition, if you use Immutable.Records on the lowest level of your state, you can inside your components access all properties similar as you would do it for objects (const record = new Immutable.Record({ foo: "bar" }); record.foo) and get all the benefits from working with immutable data structures.

How to fix the code I criticized above? The option I would prefer would be the following one by using update (the link shows the functional one, but this method exists on the data structures as well):

const increment = state => state.update("count", count => count + 1);

const decrement = state => state.update("count", count => count - 1);

(TBH, I haven’t checked it out and tested it yet, I’ll do it later 🙈 )

In the case of your example, the difference in performance etc. is not measurable. Nevertheless, sticking to best practices from the beginning (and showing them to beginners) will be thanked later when your application has grown!

I hope my comment helps you, feel free to let me know in case you need your information or let me know in case you think I’m wrong!

(EDIT The same applies to your selectors:

- export const makeCount = () => createSelector(count, count => count.toJS().count);
+ export const makeCount = () => createSelector(count, state => state.get("count");

In case a get-operation returns an immutable data structure you don‘t want to pass down to your component, perform the toJS if needed as late as possible).