High level API discussion: do we even need useMappedState

[ianobermiller]

@gaearon and I were discussing a mismatch in the model of hooks and react-redux. To quote some of our conversation:

If I'm honest, the whole useMappedState API seems flawed to me.

Redux API has mapStateToProps because it literally gives you ... props. But in Hooks you don't have a layer in the middle. You're already in render. There's no props to give.

How much does it gain you compared to direct non-mapped useStoreState()?

The idea is that useMappedState is hard to implement efficiently (it has grown pretty ugly already) and easily because it is really trying to recreate much of what React hooks already do.

The example code would basically use components as containers and selectors, similar to connect:

export default function DeleteButtonContainer({index}) {
  // Fetch state and subscribe to the store
  const state = useStoreState();

  // Map your store state to what you need, using `useMemo` or selectors
  const stateProps = useMemo(() => state.todos[index], [state, index]);

  // Callbacks
  const dispatch = useDispatch();
  const deleteTodo = useCallback(
    () => dispatch({type: 'delete todo', index}),
    [dispatch, index],
  );

  return <DeleteButton {...stateProps} {...{deleteTodo}} />;
}

const DeleteButton = React.memo(({canDelete, deleteTodo, name}) => {
  return (
    <button disabled={!canDelete} onClick={deleteTodo}>
      Delete {name}
    </button>
  );
})

And useStoreState could look something like (stripped down for brevity):

export function useStoreState<TState>(): TState {
  const store = useContext(StoreContext);
  const [storeState, setStoreState] = useState(() => store.getState());

  useEffect(
    () => {
      let didUnsubscribe = false;

      const checkForUpdates = () => !didUnsubscribe && setStoreState(store.getState());

      checkForUpdates();

      const unsubscribe = store.subscribe(checkForUpdates);

      return () => {
        didUnsubscribe = true;
        unsubscribe();
      };
    },
    [store]
  );

  return storeState;
}

Basically it just passes the entire state to the React component.

If you wanted to put them all in a single component:

export default function DeleteButton({index}) {
  // Fetch state and subscribe to the store
  const state = useStoreState();

  // Map your store state to what you need, using `useMemo` or selectors
  const {canDelete, name} = useMemo(() => state.todos[index], [state, index]);

  // Callbacks
  const dispatch = useDispatch();
  const deleteTodo = useCallback(
    () => dispatch({type: 'delete todo', index}),
    [dispatch, index],
  );

  // React will bail if the return value is reference equal
  return useMemo(() => (
    <button disabled={!canDelete} onClick={deleteTodo}>
      Delete {name}
    </button>
  ), [canDelete, deleteTodo, name]);
}

Or if you want one container and multiple children:

function Parent() {
  const state = useStoreState()
  return (
    <>
      <Child val={selectX(state)} />
      <Child val={selectY(state)} />
    </>
  )
}

const Child = React.memo(function Child({val}) {
  return <div>{val}</div>;
});

[ianobermiller]

One potential issue with this approach is that it requires either a somewhat awkward syntax (useMemo around the return) or requires more nesting in the component tree (just like connect, which we were hoping to get away from with hooks, since it has its costs).

[gaearon]

To be clear I'm not suggesting this particular API. Just that the notion of running a selector that involves props to determine a bailout has a lot of pitfalls (such as handling stale props), and can cause other issues in concurrent mode (such as the current code which mutates refs during rendering). It doesn't really map to the React conceptual model nicely. From React's point of view, props don't really exist until you render — but this Hook is trying to "bail out" at arbitrary times outside rendering.

One alternative design could be to allow selectors but only static ones:

import {createSelector} from 'react-redux-hook'

const useTodosLength = createSelector(state => state.allTodos.length)

function Todos() {
  const todosLength = useTodosLength()
  return <h1>Total: {todosLength}</h1>
}

If you need to select based on a prop value, you can do this with a component layer:

import {createSelector} from 'react-redux-hook'

const useTodos = createSelector(state => state.allTodos)

function Todos() {
  const todos = useTodos()
  return todos.map(todo => <Todo {...todo} />)
}

const Todo = React.memo(() => {
  // ...
})

Essentially, in this case the selecting component is your mapStateToProps. :-)

[gaearon]

I'd add that attempts to get rid of component nesting at all costs seem a bit misguided to me. You'll need component nesting for features like Suspense anyway. It's not an anti-pattern by itself.

Component nesting is unnecessary when there's no hierarchy. But in this case, there is. mapStateToProps pattern implies you take props from above and state from a global store, and map it to props below. I think it's rather "vertical".

In case where it's not "vertical" (hoisted selector), you don't need nesting. Like in my second proposal. But when selector itself depends on props, I think you do.

[ianobermiller]

If you need to select based on a prop value, you can do this with a component layer

This is then very similar to using a factory function for mapStateToProps. But, that doesn't work very well when you want to access that same (potentially expensive) derived state from multiple components. We use something like re-reselect in our project, so that a selector takes both state and ownProps and memoizes the result of the same state and props across calls.

[gaearon]

I also think it's tempting to underestimate problems caused by stale props. But the whole history of React Redux bindings is trying to work around this problem in various ways. This is why it's really tempting to me to not have this problem at all, at the cost of some conciseness.

that doesn't work very well when you want to access that same (potentially expensive) derived state from multiple components.

This kind of API can probably be extended to handle that too. I don't see why not — as long as props don't appear in the middle.

[markerikson]

The question of what a "proper" React+Redux+hooks API should look like is interesting. But, in all honesty, I haven't dug into it seriously yet myself, because we need to work through other issues with React-Redux first before we can deliver a public hooks-based API (per reduxjs/react-redux#1177 ).

We do have an open thread for bikeshedding potential hooks API designs at reduxjs/react-redux#1179 . It's got some discussion, but I haven't paid attention to it yet.

I agree that with hooks, you can do a lot of the extraction / memoization work right there in the component itself.

However, one of the key objectives of React-Redux has always been to keep the app performant, especially given Redux's global update subscription behavior. Dan and I have talked about this a few times, and I know he and I don't see eye to eye on this, but:

All the benchmarks and investigation we've done show that doing the comparisons and extraction first, and only triggering a re-render if the derived data has changed, ultimately results in better performance, because React only has to re-render when there's an actual change. Forcing re-renders for every state change, regardless of whether there's a component that will actually update, is always going to be more overhead.

I do definitely agree that the "stale props" issue is a major aspect to take into consideration overall.

[ianobermiller]

All the benchmarks and investigation we've done show that doing the comparisons and extraction first, and only triggering a re-render if the derived data has changed, ultimately results in better performance, because React only has to re-render when there's an actual change. Forcing re-renders for every state change, regardless of whether there's a component that will actually update, is always going to be more overhead.

This has been my experience as well. Basically staying out of React as long as possible yields the best performance. But, I would love to be proven wrong on this.

I also think it's tempting to underestimate problems caused by stale props. But the whole history of React Redux bindings is trying to work around this problem in various ways. This is why it's really tempting to me to not have this problem at all, at the cost of some conciseness.

I do definitely agree that the "stale props" issue is a major aspect to take into consideration overall.

This is one of those issues that a library either has to solve for you or avoid entirely, since it isn't a problem until it is, and then it is a big problem and hard to diagnose.

[gaearon]

What's a good benchmark? @markerikson

[markerikson]

Well, what we've got atm is the React-Redux benchmarks suite at https://github.com/reduxjs/react-redux-benchmarks . I'm not going to claim those are "good", necessarily, but it's at least something that provides concrete objective numbers to compare with.

[dai-shi]

I would like to share that I tried running one of benchmark scenarios by @markerikson to compare this library and react-redux, and some others.
dai-shi/reactive-react-redux#3 (comment)

[ghost]

@ianobermiller

just like connect, which we were hoping to get away from with hooks, since it has its costs

Using connect() in combination with function components seems such a simple and straightforward solution with a neat API. I wonder what the costs and pitfalls of using connect are. Should I avoid it? What's the rationale here?

[ianobermiller]

Using connect() in combination with function components seems such a simple and straightforward solution with a neat API.

It definitely depends on your app and would require careful profiling. We found that swapping out connect and other HOCs for hooks with a flatter tree did result in some substantial perf gains when rendering thousands of items. But connect was probably only a small part of that, so YMMV.

[ianobermiller]

Closing this out for now since I don't think we are going to drastically change the API.

[markerikson]

@ianobermiller : out of curiosity, how much use is this library still getting now that React-Redux ships an "official" hooks API?

[ianobermiller]

My guess is as good as yours. Judging by NPM stats, people are still using it, but it isn't growing much. We use it internally for a few products at Facebook. My sense is that there is still a demand for a lighter weight way to use hooks with Redux. Do you see react-redux offering a hooks-only version in the future?

[markerikson]

I don't see us creating a separate package, if that's what you're asking.

As far as I know, the vast majority of React-Redux usage is still connect, and we have no plans to deprecate that in any way.

That said, I could imagine that some apps might only actually want to use the hooks. The better question is whether connect tree-shakes or not if it's unused, and that's not something we've explored.

I just opened up an issue to investigate whether we can tree shake or not:

reduxjs/react-redux#1470