Inserter: Use effect for unmodified default block replacement

[aduth]

Related: #4917

This pull request seeks to refactor the inserter default block replacement logic introduced in #4917 with an effect-based implementation. This optimizes the rendering of Inserter which currently rerenders on any change to the selected block, since the mergeProps result includes a changing function reference (even though we're careful not to pass the changing selectedBlock reference).

Testing instructions:

Repeat testing instructions from #4917

[youknowriad]

We could use limit the prop to the uid as well.

[youknowriad]

I think I prefer the previous code (but I'm biased since I wrote it) but I don't feel strongly one way or another.

[youknowriad]

I like that it's a separate documented function though it creates inconsistency between the effect handlers, we might want to move them all to separate functions.

[aduth]

I like that it's a separate documented function though it creates inconsistency between the effect handlers, we might want to move them all to separate functions.

Yeah, this was trying something new, since I didn't like the disorganization of this file, which is largely a result of a disconnect between the action types upon which they act, and the behaviors themselves. This had also encouraged including many different side effects in the same handler, when in-fact the value can be an array of functions, where each is an individual side effect for that particular action. Finally (and a more minor point), our unit tests should be testing the behavior of the side effect, not the original firing action.

[aduth]

I think I prefer the previous code (but I'm biased since I wrote it) but I don't feel strongly one way or another.

I'm on the fence myself whether this behavior should be best described as a side-effect. The more pragmatic concern is that of performance: re-rendering this component on every block update.

[aduth]

The more pragmatic concern is that of performance: re-rendering this component on every block update.

In a worst case, there's an areMergedPropsEqual function we could implement in the fourth options argument of connect, though this would be tricky to do correctly since we do need the function to change when the values it depends on change.

https://github.com/reactjs/react-redux/blob/master/docs/api.md#connectmapstatetoprops-mapdispatchtoprops-mergeprops-options

[aduth]

The component still renders more than it should (because of the selectedBlock prop), but this has been pretty well improved using withDispatch in #5137.