React-redux: Feedback: React-Redux v7 hooks alpha

[markerikson]

Please use this thread to give feedback on the new React-Redux hooks APIs available in 7.1-alpha.

Prior references and discussion:

• Docs (next): Hooks
• issue #1179: Discussion: Potential hooks API design
• PR #1248: Add React hooks

[markerikson]

Copying over comments from the other couple threads for reference:

https://github.com/reduxjs/react-redux/issues/1179#issuecomment-485362870 (@mungojam ):

Very cool to see this coming into shape.

The new docs don't look quite right for the useCallback with useSelector example (though useCallback vs. useMemo isn't fully in my head yet):

const todoSelector = useCallback(() => {
    return state => state.todos[props.id]
}, [props.id])

const todo = useSelector(todoSelector)

It seems like the useCallback is actually being used as if it were useMemo, when it should just be:

const todoSelector = useCallback(
  state => state.todos[props.id], 
  [props.id]
)

const todo = useSelector(todoSelector)

The equivalent useMemo version would be as per the current example:

const todoSelector = useMemo(() => {
    return state => state.todos[props.id]
}, [props.id])

const todo = useSelector(todoSelector)

[markerikson]

https://github.com/reduxjs/react-redux/issues/1179#issuecomment-485362870 (@mungojam ):

Sorry if I've missed the answer to this earlier, but what is the idea behind having the dependencies array in useAction, but not in useSelector? It would be nice if useSelector had it too as that feels more hooks-idiomatic and we wouldn't then need to use useMemo or useCallback if we include props in it.

[markerikson]

https://github.com/reduxjs/react-redux/issues/1179#issuecomment-485368262 (@MrWolfZ ):

@mungojam You didn't miss anything. It was indeed an oversight on my part that I noticed myself over the weekend, as discussed in this post. I think it makes perfect sense to add the deps to useSelector, but it might make useRedux a bit more tricky.

[markerikson]

https://github.com/reduxjs/react-redux/pull/1248#issuecomment-485328604 (@markerikson ):

A couple quick observations as I work on docs:

• useRedux() doesn't allow you to pass a deps array for actions

• We probably oughta tweak the error message for useReduxContext() a bit

  • Do we need that same "didUnsubscribe" check in useSelector() that we have in connect()?

  • Do we want to make an empty deps array the default for useActions()? How often would you actually want to recreate those every time?

[markerikson]

https://github.com/reduxjs/react-redux/pull/1248#issuecomment-485335594 (@MrWolfZ ):

@markerikson I worked a bit with the hooks on the weekend, and also made an observation. Currently, the selector in useSelector is re-created every time. That means if you use a memoizing selector you need to memoize the selector itself yourself. This could be fixed by also passing a deps array into useSelector.

Now my comments to your observations:

useRedux() doesn't allow you to pass a deps array for actions

Indeed, I completely missed this. It would be easy enough to add this parameter, but if we decide to add deps to useSelector as mentioned above, then things get messy. Do we only use a single deps for useRedux? Or pass in two of them? I think this is an argument for removing this hook after all.

Do we need that same "didUnsubscribe" check in useSelector() that we have in connect()?

I intentionally removed this, since it was only required to ensure the selector never sees inconsistent state and props. Basically, with my implementation it does not matter whether the subscription callback is called after the component was unmounted. The only thing that happens is potentially a forceRender call that does nothing. Also, based on this issue the didUnsubscribe guard doesn't seem to be enough to prevent the callback to be called after unmounting anyways. On the flip side it doesn't hurt to have that check in there and it could give minimal performance improvements by not calling the selector when we know we don't need to. I prefer having a simpler implementation (and not having to write a test for it ;) ), but feel free to add it back in.

Do we want to make an empty deps array the default for useActions()? How often would you actually want to recreate those every time?

I strongly argue to not make an empty deps array the default. Firstly, to be consistent with how useEffect, useMemo, and useCallback work. Secondly, if we were to do this, it might lead to very subtle bugs with stale props inside the actions that are incredibly hard to debug. By leaving the default undefined the worst thing that can happen is slightly worse performance and that can easily be fixed by just adding the deps yourself.

[vadzim]

As I understand the main reason to have the object form of useActions is to use all the actions from a module like

import * as actions from 'myactions'
...
const { action1, action2 } = useActions(actions)
...

I see a possible disadvantage here that statically analyzing tools will not be able to understand that some of actions are not used in a project anymore. E.g. WebStorm notifies about unused exports in a project.

[mungojam]

Moving my comment https://github.com/reduxjs/react-redux/pull/1248#issuecomment-485371191

const boundAC = useActions(actionCreator : Function, deps : any[])

const boundACsObject = useActions(actionCreators : Object<string, Function>, deps : any[])

const boundACsArray = useActions(actionCreators : Function[], deps : any[])

Unless there is a compelling reason for both, I think it would be good to drop either the 2nd or 3rd version of useActions. I'd rather have just one way to create multiple actionCreators.

I probably prefer the object one as it is more auto-documenting within the hook. The hooks that use arrays like useState etc. are different because it is for a single thing and the two array elements usually have a single name i.e. [count, setCount]. It also lets users name them easily rather than {state: count, setState: setCount}. You don't have that problem with useActions.

[mungojam]

Moving my comment https://github.com/reduxjs/react-redux/pull/1248#issuecomment-485372560

Do we only use a single deps for useRedux? Or pass in two of them? I think this is an argument for removing this hook after all.

Personally I think useRedux isn't very hooksy. Either people will write their action creator and selector logic within the useRedux call and then it's quite verbose, or they will write them outside and then what is actually being usefully added by useRedux? Maybe there are some more advanced scenarios it could cater for if added later but I don't think it should be added if it's just for familiarity with connect as it's just an added layer over the other hooks.

[threehams]

@MrWolfZ Are you planning to maintain a PR at https://github.com/DefinitelyTyped/DefinitelyTyped? If not, I can add one later tonight and maintain it with alpha changes.

[Satyam]

In useSelector, in line 55, where it says:

const memoizedSelector = useMemo(() => selector, deps)

Wouldn't it make sense to have:

const memoizedSelector = useMemo(() => () => selector(deps) , deps)

so you could do:

const todo = useSelector(([id]) => state => state.todos[id], [props.id])

I would also love to have the option of using a path for a selector, much like lodash _.get() but using placeholders for dependencies, thus:

const selTodoItem = ([id]) => state => state.todos[id];

Would turn into:

const selTodoItem = 'todos.%0';

[MrWolfZ]

@threehams thanks for reminding me. I have created a PR with the hooks types. Any feedback is highly welcome, since it is the first time I am contributing to DefinitelyTyped.

@Satyam what is the benefit of getting the props into the selector as an argument instead of just by closure? One downside would definitely be more typing. Regarding the path, I feel that is a very special case that you can easily create a custom hook for. One of the points of hooks was to make it easy to compose them and to make it easy to create your own. I am not the maintainer of this repo, so I don't have the last say on this, but I definitely feel we should leave the hooks as simple as possible and not overload them too much.

[Satyam]

@MrWolfZ The reason for having the selectors get the props, or whatever other dependency, as an argument is that I don't usually mix details of the redux store along the components. I have the selectors defined somewhere along the store so that if the store changes in any way, I don't have to go searching for all the affected selectors embedded within the components. Likewise, if the same selector is used in several places, I don't have to copy and paste the same selector code into the several components that use it. And it is not the component props that I mean to use as arguments for the selector but any value taken from wherever it might be. The arguments used by the selector and its dependencies are one and the same, I can hardly imagine a selector that has more dependencies than arguments or vice versa.

Also, I don't see why those arguments are to be passed as an array of dependencies, it might be better to have it as an object so the arguments can be named. The fact that they are also dependencies in the memoized selector should be an implementation detail solved internally, for example:

const memoizedSelector = useMemo(() => () => selector(deps) , Object.values(deps))

So, the selector could be called:

const todo = const todo = useSelector(selTodoItem, { id: props.id});

where selTodoItem would be defined elsewhere as :

const selTodoItem = ({id}) => state => state.todos[id];

which clearly is not such an improvement over my previous example with just one argument passed as an array, but it would make things clearer if there were more than one argument.
So, basically, the principle of separation of concerns is what makes it funny to have the action creators and the reducers all close to the store, but the selectors along the components, just to leverage the closure.

[markerikson]

@Satyam : I'm going to have to disagree with several of your opinions here:

• I don't understand your proposal for const todo = useSelector(([id]) => state => state.todos[id], [props.id]) at all
• We're definitely not going to add some kind of a string interpolation overload. If you want to generate selectors along that line, it should be easy enough to do yourself and pass them to useSelector
• Dependency arrays are the natural form here because that's how useMemo and useCallback work. There's no reason to ask users to define them as an object, only to promptly convert the values back into an array.
• If you want to pre-memoize the selector function yourself, you can do that. But, given the common case of selecting values based on props, it's reasonable for us to offer an API that just handles that automatically if the deps are supplied.

[MrWolfZ]

@Satyam the Object.values approach won't work, since it uses the same order as for...in and that order is arbitrary and not necessarily stable, as you can see here.

Regarding passing args to the selector, I assume the most common use case will be inline selectors that can just use closure. However, even for your case you can just do this:

const todo = useSelector(selTodoItem({ id: props.id}), [props.id]);

If you really want to have such a hook (and don't care about stability of props order), it is easy to implement it:

const useMySelector = (selector, deps) => useSelector(selector(deps), Object.values(deps))

[Satyam]

@markerikson

@Satyam : I'm going to have to disagree with several of your opinions here:

• I don't understand your proposal for const todo = useSelector(([id]) => state => state.todos[id], [props.id]) at all

The reason for const todo = useSelector(([id]) => state => state.todos[id], [props.id]) is that the selector can be defined elsewhere, more likely along the store itself, like shown later: const selTodoItem = ([id]) => state => state.todos[id];. As the argument id comes as an argument, it doesn't rely on closure which requires theselector to be defined within the component.

• We're definitely not going to add some kind of a string interpolation overload. If you want to generate selectors along that line, it should be easy enough to do yourself and pass them to useSelector

The string interpolation was just a wish list item of mine, I shouldn't have mentioned it along the main issue, sorry.

• Dependency arrays are the natural form here because that's how useMemo and useCallback work. There's no reason to ask users to define them as an object, only to promptly convert the values back into an array.

It feels like the external API is being defined by the internal implementation of the API and not by the way a developer might use it. The developer should not be concerned if the API uses useMemo or whatever else internally. It should not be defined because useMemo expects dependencies, it should be defined because the selector needs arguments which happen to be its dependencies.

• If you want to pre-memoize the selector function yourself, you can do that. But, given the common case of selecting values based on props, it's reasonable for us to offer an API that just handles that automatically if the deps are supplied.

Actually, when I wrote that part, the documented API didn't have the deps argument yet so it no longer applies.

@MrWolfZ That is true, redundant as it is. If the deps are the arguments to the selector, why not giving them to it? I don't know the reason why useMemo doesn't provide the memoized function with the arguments but it clearly is something a developer would expect. The way the issue is highlighted in the documentation clearly shows that it is contrary to normal expectation. I would assume that there is some technical reason for that in useMemo, I don't see any reason not to have it in useSelector and avoid the unnecessary repetition or tying up the selector to the component by closure.

[Satyam]

@MrWolfZ Besides, if you do it the way you suggest:

const useMySelector = (selector, deps) => useSelector(selector(deps), Object.values(deps))

What is the point of memoizing if the part that does depend on the arguments, that is selector(deps) has already been evaluated?
Anyway, my point in passing arguments as an object was to show that there are many ways to define the API for useSelector that are better oriented to the developer using it and are not so much focused on the way useMemo works.

[ricokahler]

@MrWolfZ these look amazing! Nice job!

Here are my two cents:

I worked a bit with the hooks on the weekend, and also made an observation. Currently, the selector in useSelector is re-created every time. That means if you use a memoizing selector you need to memoize the selector itself yourself. This could be fixed by also passing a deps array into useSelector.

I think the deps array should be a required argument, it makes things faster and isn't much of a mental lift for the user to add. The current typings for useMemo also require the deps array so I think it wouldn't be a bad idea to make that argument required and invariant if it's not there.

---

https://github.com/reduxjs/react-redux/blob/87841fa2ce62a4221e8803f32e7c6282fe086111/src/hooks/useSelector.js#L91

I wonder if this would be better as an Object.is or === check here. I've messed with some redux hooks and I've always used multiple useSelector calls vs returning an object e.g.

function Foo() {
  // returning an object where shallowEqual helps
  const { a, b } = useSelector(state => ({ state.a, state.b }), []);

  // using multiple calls to `useSelector`
  const a = useSelector(state => state.a, []);
  const b = useSelector(state => state.b, []);
  // i like doing this 👆better
}

I like this better because it feels simpler. If you're returning part of the store without any calculations or creation of objects/arrays, you don't need to shallowEqual. It also feels more hook-esque to call useSelector more than once.

In cases where I would want to create an object within the selector, I like to pass in a memoized function e.g.

import { createSelector } from 'reselect';

const selectA = state => state.a;
const selectB = state => state.b;
const selectAB = createSelector(selectA, selectB, (a, b) => ({ a, b }));

function Foo() {
  const ab = useSelector(selectAB, []);
}

and then memoized function would return the same object reference not needing the shallow equal check either.

Maybe that's more complicated? but it fits in with the current Redux ecosystem so I'm okay with it.

---

In regards to Action Object Hoisting, I've actually found it to be relatively ergonomic to just useDispatch and wrap all action calls in dispatch.

function Foo(personId) {
  const dispatch = useDispatch();

  useEffect(() => {
    dispatch(fetchPerson(personId));
  }, [personId]);

  // ...
}

The only time where I found it to be more ergonomic to use something like useActions is when passing abound version of an action creator to a lower components. I've done it like this:

function Container() {
  return <PresentationalComponent addTodo={useAction(addTodo)} />
}

However, there is another idea I've had inspired by @material-ui/styles:

In material-ui/styles, they have a factory makeStyles that returns a hook useStyles

import React from 'react';
import { makeStyles } from '@material-ui/styles';

const useStyles = makeStyles({
  root: {
    backgroundColor: 'red',
  },
});

export default function MyComponent() {
  const classes = useStyles();
  return <div className={classes.root} />;
}

Similarly, we could create a factory makeAction that will wrap an action creator and return a hook:

import React from 'react';
import { makeAction } from 'react-redux';
//                               👇 could also pass in an existing action creator
const useAddTodo = makeAction(todoName => ({ type: 'ADD_TODO', payload: todoName }));

function Foo() {
  const addTodo = useAddTodo();

  // ...
}

Maybe that's more verbose but I like the idea and conventions of it. Let me know what you think.

[josepot]

Hi and thanks a lot for this @MrWolfZ and @markerikson !

I like it, a lot!

Although, I agree with @ricokahler in that I would prefer for useSelector to perform an Object.is or a === instead of a shallow-compare. I understand that the first thing that the shallowCompare does is to check for referential equality, and that the only difference that it would make for me would be the few unnecessary comparisons that will take place when the result of the selector has actually changed... But still, I would much rather if useSelect didn't perform a shallow-compare by default... Perhaps that could be an option? or another hook useObjectSelector?

Finally, about the "controversy" on whether useSelector should accept props or not. I like the fact that it doesn't. However, if Reselect v5 proposal got accepted, perhaps it would make sense to include a different hook named useInstanceSelector (or something like that) that could accept props and "usable selectors". Although, I understand that could be seen as coupling this API to the API of an optional dependency, so I can see how that could be controversial... Although, I still think that it would be an idea worth considering.

Thanks again for this!

[MrWolfZ]

@Satyam

@MrWolfZ Besides, if you do it the way you suggest:

const useMySelector = (selector, deps) => useSelector(selector(deps), Object.values(deps))

What is the point of memoizing if the part that does depend on the arguments, that is selector(deps) has already been evaluated?

I'm not sure I understand. Yes, a new result for selector(deps) is always created, but the callback that is memoized and then used to select from the state is the first one that was created for each deps. So if selector memoizes on either the deps, the state or both it will work.

Anyway, my point in passing arguments as an object was to show that there are many ways to define the API for useSelector that are better oriented to the developer using it and are not so much focused on the way useMemo works.

I'm just gonna say that your suggestion is oriented towards how _you_ are using the API, but I am sure there are loads of other ways to use it we can't even think of. The idea behind making it work the same as useMemo and useCallback is to a) make it immediately familiar to all react hooks users, and b) to build upon the foundations that the react team built that they surely have put some thought into. Also, with your suggestion, the semantics of the first parameter would change based on whether the second parameter was passed (i.e. we would probably need different handling for no deps, empty deps, and filled deps). This makes the API very complex and also makes it difficult to add the deps parameter later on. I completely understand where you are coming from with this proposal, but in the end, there is no right answer to API design and this mostly comes down to personal preference, and I prefer to stick with the API I implemented.

@ricokahler

I think the deps array should be a required argument, it makes things faster and isn't much of a mental lift for the user to add. The current typings for useMemo also require the deps array so I think it wouldn't be a bad idea to make that argument required and invariant if it's not there.

I don't mind making it mandatory, but I also don't strongly feel that it needs to be. The only downside of not providing it would be potentially worse performance and the fix to that is easy. I also think this case is slightly different to useMemo in that useMemo would be completely pointless without the deps but useSelector works just fine without it.

I wonder if this would be better as an Object.is or === check here.

Yeah, I thought about this as well. First of all, even with the current implementation nothing keeps you from calling useSelector multiple times. Secondly, the downside of not doing shallowEquals is that useSelector(state => ({ a: state.a, b: state.b })) is now always causing a render and the fix to that would be not very obvious. I think that usage pattern will be common enough to justify doing shallowEquals. Lastly, I have actually benchmarked the hooks both with reference equality and shallowEquals and the results were almost identical, even though the benchmarks exactly trigger the sub-optimal behaviour by selecting large arrays from the state where each value is compared with shallowEquals (sadly I didn't keep the results around but you can easily reproduce this if you want).

Similarly, we could create a factory makeAction that will wrap an action creator and return a hook:

Yup, I have thought about this as well and I am actually doing something similar in a (toy) project I work on. That said, the current API does in fact already allow you to do this easily. Just do this:

const useAddTodo = () => useActions(todoName => ({ type: 'ADD_TODO', payload: todoName }))

If you find that unwieldy you can always easily create makeAction yourself:

const makeAction = ac => () => useActions(ac)

@josepot

Finally, about the "controversy" on whether useSelector should accept props or not. I like the fact that it doesn't. However, if Reselect v5 proposal got accepted, perhaps it would make sense to include a different hook named useInstanceSelector (or something like that) that could accept props and "usable selectors". Although, I understand that could be seen as coupling this API to the API of an optional dependency, so I can see how that could be controversial... Although, I still think that it would be an idea worth considering.

I have to admit I am not very familiar with reselect and the likes. However, all the examples I see at reselect and also in redux-views have selectors that depend on how mapStateToProps works, i.e. taking state and props as arguments. With hooks you can actually achieve the same by just using closure (basically, by doing what I suggested above already):

const getUsers = state => state.users

const getUser = id => createSelector(
  getUsers,
  users => users[id]
)

const user = useSelector(getUser(props.id), [props.id])

In that example, useSelector takes care of memoizing on the props, while reselect takes care of memoizing on the users. However, as I said I am not so familiar with this approach, so please let me know if I made a mistake in my way of thinking.

[markerikson]

FWIW, it's always possible for us to add more hooks down the road, so if anything I would prefer to keep the initial list relatively minimal and primitive. On the flip side, I also don't want to have to go _changing_ the hooks we ship (no more major versions!), so we want to get these initial APIs correct before they go final.

[janhesters]

Don't know if this is the right place for mistakes in the docs and whether this is indeed an error, but in the next docs it says:

const increaseCounter = ({ amount }) => ({
  type: 'increase-counter',
  amount
})

export const CounterComponent = ({ value }) => {
  // supports passing an object of action creators
  const { increaseCounterByOne, increaseCounterByTwo } = useActions(
    {
      increaseCounterByOne: () => increaseCounter(1),
      increaseCounterByTwo: () => increaseCounter(2)
    },
    []
  )

increaseCounter(1) would throw, because 1 has no property called amount

I think this should be:

increaseCounterByOne: () => increaseCounter({ amount: 1 }),
increaseCounterByTwo: () => increaseCounter({ amount: 2 })

or

const increaseCounter = amount => ({
  type: 'increase-counter',
  amount
})

I might be wrong here though.

[G710]

Hi,

first of all, thank you for your hard work and thoughts that went into this library (and this new API in particular :+1: )

I've been experimenting with the lib today and so far I really like it! The provided hooks fit perfectly into the new eco-system and feel natural to use. I've never really warmed up to the mapXToProps functions but the hooks just make sense to me.

I don't really get the fuzz about the useSelector deps. I mean, you can pass props to the function in it's current state, right? (seems to be working in my project) You just have to be careful that your selector can't run into an invalid state.

As far as I understand most suggestions so far can be build upon the existing functions and are easy enough to a) just implement in the project or b) write up a small lib.

Let's see if I run into any issues in the next few days but so far the transition seems to be seamless. Thanks a lot! :+1:

[markerikson]

@janhesters : yep, good catch. If you've got time, please submit a PR to fix those examples in the docs on master and in the source comments in v7-hooks-alpha (which is where I copied them from).

@G710 : yes, you can totally use props in selectors, it's just that there's certain cases you have to watch out for. Thanks for the feedback!

[janhesters]

@markerikson Sure. Which of the two fixes do you want me to create a PR for?

A:

increaseCounterByOne: () => increaseCounter({ amount: 1 }),
increaseCounterByTwo: () => increaseCounter({ amount: 2 })

or B:

const increaseCounter = amount => ({
  type: 'increase-counter',
  amount
})

[markerikson]

I'd say change amount from a destructured object to a simple number, to match the other usages (increaseCounter(2)).

[janhesters]

PR for master.

But I can't find hooks.md in the v7-hooks-alpha branch 🤔

Do you mean the hooks-docs branch?

[markerikson]

Correct - due to the docs setup, the docs themselves are on master, even though we've got the code over in that v7-hooks-alpha branch.

[MrWolfZ]

@janhesters thanks for pointing out the mistake. I actually copied the mistake in the typescript typings as well, and your comment allowed me to fix it in my PR. Before you create the PR for the inline docs, I actually made some adjustments in that PR that I'd like to copy into the inline comments. Therefore, I'll create a PR shortly to update the inline comments.

EDIT: PR is here.

[janhesters]

@markerikson PR for hooks-docs.

[markerikson]

Ok, merged the PRs for the docs on master and the JSDocs in v7-hooks-alpha. The hooks-docs branch is OBE and needs to be deleted.

[josepot]

FWIW, it's always possible for us to add more hooks down the road, so if anything I would prefer to keep the initial list relatively minimal and primitive. On the flip side, I also don't want to have to go changing the hooks we ship (no more major versions!), so we want to get these initial APIs correct before they go final.

:100: to that! In fact, since hooks are composable, it would be trivial to create the useInstanceSelector hook that I was mentioning just by enhancing the original useSelector. Something like this should do it:

const useInstanceSelector = (selector, props) => {
  const { keySelector, use } = selector

  const key = useMemo(
    () => keySelector ? keySelector(null, props) : props,
    [keySelector, props]
  );
  useEffect(() => use && use(key), [use, key]);

  const selectorWithProps = useCallback(
    state => selector(state, props),
    [key]
  );
  return useSelector(selectorWithProps, [key]);
};

So, if v5 of Reselect makes it, then perhaps we could consider adding something like this... Or not... it could also be added from a different library. So yeah, I totally agree that it's best to get a good API first :+1:

@MrWolfZ What you are proposing would only work for "factory selectors", for sure, but not for "cached selectors". The problem with "factory selectors" is that they are a pain to compose, and also it's not possible for 2 different "factory selectors" to share the same "cache"... that's why I have created Redux-Views and why I'm making the proposal for Reselect v5. But no worries, since I already said in my first message, I think that it's a good thing that useSelector doesn't accept props :smile: It's better to have basic hooks that we can compose with, than to have overloaded monsters that try to cluster too much functionality.

Edit:

Oh! I forgot to mention, that an addition like this wouldn't require a major upgrade... It would be added functionality, no need for a major upgrade because it would be 100% backward compatible, just a minor upgrade would suffice.

[threehams]

Are we sure we want to have useRedux? I'm writing tests for the TypeScript library definitions, and I'm finding the usage to be really clunky in practice when compared to separated useSelector and useActions.

It's not especially hard for code or types, just seems like another way to do things and doesn't serve much of a purpose, besides familiarity with connect().

edit: By "clunky" I mean that as I'm writing the tests, I'm continually getting the expectations wrong due to the combination of nested arrays, objects, arrays + object, and single items. Destructuring is definitely making the problem worse.

// not bad for simple cases
const [state, boundAction1] = useRedux(selector, actionCreator1);

// ehhhhhhh
const [
  { counter },
  [
    boundAction1,
    boundAction2,
    boundThunk,
  ]
] = useRedux(selector, [actionCreator1, actionCreator2, thunkActionCreator]);

compared to separate hooks, where the object returned directly matches what was supplied:

const { counter } = useSelector(selector);
const [
  boundAction1,
  boundAction2,
  boundThunk,
] = useActions([actionCreator1, actionCreator2, thunkActionCreator]);

[wangtao0101]

@markerikson One question:
useSelector seems not to update the context with new subscription, then how to enforce top-down updates

[MrWolfZ]

Are we sure we want to have useRedux? I'm writing tests for the TypeScript library definitions, and I'm finding the usage to be really clunky in practice when compared to separated useSelector and useActions.

I am also in favour of removing useRedux.

[MrWolfZ]

@wangtao0101

useSelector seems not to update the context with new subscription, then how to enforce top-down updates

Indeed, with useSelector we do not require top-down updates anymore. I have already explained this in the API design issue, so I'll just move over the post.

My proposal simply accepts that there will be cases in which props and state are out of sync inside the subscription callback and defers computing the final value to the render phase. This all hinges on the assumption that the selector is pure.

Let's look at the stale props scenario. A state update occurs, which causes the props and state to change. The child callback is executed first with the old selector (which may close over stale props), and may or may not cause a re-render (depending on what the stale props cause the selector to return). However, since a props change occurs, the component will re-render regardless of whether the subscription callback forced it or not. Since v7 uses batched updates, the component will not be re-rendered immediately, even if the callback forces it, but only after any parents have processed the store update as well. Then, during render we re-apply the selector with the latest props. The wrong result of applying the selector in the subscription callback does not matter at all at this point since it was used only as a heuristic to determine whether a render should be triggered.

The only edge case is when the stale props cause the selector to throw. My suggestion simply ignores errors when applying the selector in the subscription callback and forces a re-render. If the component gets unmounted before the render occurs, nothing happens. If it gets re-rendered the selector is applied again. Since it is pure (as per my assumption above) if the condition which caused the error in the callback (be that stale props or whatever) is still present, it will throw again (and allow error boundaries etc. to handle the error). However, if something changed, it will just work.

Personally, I can't come up with any case in which you would care about those hidden errors. However, one option I considered is to log the error in development mode (and allow suppressing it once a dev looked at the cause and determined it was harmless, e.g. useRedux(state => state.items[id], { suppressErrorsInSubscriptionCallback: true })).

In fact, the example sandbox I posted has the stale props situation and works just fine. This fork logs the error, so you can see what is happening.

@markerikson maybe we can add a short section about this to the release notes or the docs so that people don't get confused why a HOC is not required anymore?

[markerikson]

@wangtao0101 , @MrWolfZ : I did try to address this in the docs already:

https://react-redux.js.org/next/api/hooks#stale-props-and-zombie-children

If you feel the current description isn't sufficient, then yeah, we can totally update the docs accordingly. File a PR with your suggestions.

[markerikson]

As for useRedux(): yeah, the primary reason to have it is familiarity with connect(). I agree it's probably not too hard to say "to migrate, just call useSelector() _and_ useActions()" and drop it.

That said, the other question is how much we would expect people to be destructuring the return values in practice.

[ThinkSalat]

I got a bug saying could not find react-redux context value; please ensure the component is wrapped in a <Provider>

The rest of redux works, but the useSelector call is what's causing this error.

Here's the component in question.
const ReduxHookTest = props => { let hasCBT = useSelector(patientSelectors.getEvents) console.log(hasCBT); return ( <div></div> ) }

[markerikson]

@thinksalat : that sounds like some kind of a configuration issue. Can you put together a project that reproduces the problem?

[timdorr]

I'd also say we should remove a connect analogue because of the zombie child behavior difference. useRedux sets the wrong expectations for the user. Very sadness; much bad.

[ThinkSalat]

@ThinkSalat : that sounds like some kind of a configuration issue. Can you put together a project that reproduces the problem?

I tried and I can't reproduce it. It's weird that redux is working in all situations except when I try to use a hook. I have this test component inside of a component that's connected to the store using connect.

I tried changing my root to return this :
return ( <Provider store={store}> <ReduxHookTest /> </Provider> )

and it still gets the error.

[MrWolfZ]

@ThinkSalat hmm, with react there have been situations reported where two versions of react got bundled with the app. Maybe your case is similar? If the Provider uses a different ReactReduxContext than the useSelector hook, this error could occur. Usually this is due to non-standard bundling processes where multiple node_modules folders are used. Do you have anything like this in your app?

@markerikson regarding the zombie child problem in the docs: you give a lot of advice of what not to do with useSelector even though all those things will work just fine. As long as the selector is pure, it shouldn't matter at all whether the props are stale or not. That's the whole point of the way I implemented it. Specifically this statement is not required IMO:

In cases where you do rely on props in your selector function and those props may change over time, or the data you're extracting may be based on items that can be deleted, try writing the selector functions defensively. Don't just reach straight into state.todos[props.id].name - read state.todos[props.id] first, and verify that it exists before trying to read todo.name.

[G710]

@ThinkSalat I had the same error message earlier today. In my case it also was for a component inside a component, but in my project both are connected via hooks inside a Provider.

In my case I think it was a faulty import statement due to VSCode auto-import generator. Maybe check, if your import really looks like this: import { useSelector, useActions } from "react-redux". I also did something with my useSelector calls but I think that was another story.

Maybe this working minimal example can help you find your error (I also did the whole component-in-component thing to show the context works as expected): https://codesandbox.io/embed/2znoz180nj

[ThinkSalat]

@ThinkSalat hmm, with react there have been situations reported where two versions of react got bundled with the app. Maybe your case is similar? If the Provider uses a different ReactReduxContext than the useSelector hook, this error could occur. Usually this is due to non-standard bundling processes where multiple node_modules folders are used. Do you have anything like this in your app?

We only have one node_modules folder, unless you count the node_modules folders inside of the packages themselves. I did just upgrade from React 16.8.1 => 16.8.

@G710 My imports were fine, I was importing like import { connect, useSelector } from 'react-redux';

Again I'm really confused by why connect would work, but useSelector doesn't.

[dmytro-lymarenko]

What do you think about this:
in the place where we define our action creators we also create hooks related with them like this:

function createActionCreatorHook<R extends Action, Args extends any[]>(
    actionCreator: (...args: Args) => R
): () => (...args: Args) => R {
    return () => {
        const dispatch = useDispatch();

        const dispatchedActionCreator = useCallback(
            (...args: Args) => dispatch(actionCreator(...args)),
            [dispatch, actionCreator]
        );

        return dispatchedActionCreator;
        // or just
        // return useActions(actionCreator, []);
    }
}

and then for any action creator we write this:

// action creators
export const increaseCounter = amount => ({
    type: 'increase-counter',
    amount
})
...
// hooks
export const useIncreaseCounter = createActionCreatorHook(increaseCounter);

This alows us to import only hooks in component like this:

// import { useIncreaseCounter } from 'actions';
function CounterPresenter() {
    const increaseCounter = useIncreaseCounter();
    ...
    const onClick = useCallback(() => { increaseCounter(4); }, [increaseCounter]);
    reutrn (...);
}

[janhesters]

@dmytro-lymarenko Isn't useIncreaseCounter() the same as useAction(increaseCounter)?

And if you wanted to reuse that you could make a custom Hook:

function useIncreaseCounter() {
  return useAction(increaseCounter);
}

[dmytro-lymarenko]

@janhesters Yes, they are the same. My main idea was that we can keep action creators and hooks in the same place and import only hooks inside our components.

[Satyam]

After toying with several alternatives for useSelector, some of which I unwisely speculated over openly earlier (sorry about that), I settled on this one which is not far from the original, with only a few lines changed.

I have useSelector declared like this:

export function useSelector(selector, ...args) {

And the memoization of the selector is changed to this:

// eslint-disable-next-line react-hooks/exhaustive-deps
const memoizedSelector = useMemo(() => selector(...args), [selector, ...args])

The selector and the values in the arguments are used as dependencies of the memoization. The disabled eslint rule is because otherwise it changes the ...args dependencies to arg, thus depending on the array (which is new each time) instead of its values.

Selectors can be defined like this:

export const selUsers = () => state => state.users.data;
export const selUser = id => state => state.users.data[id];

The developer is not forced to define the selectors in-situ as all the information they need is passed as arguments.

It is worth noting that the selectors don't receive the component properties wholesale, only the values they need for their job, for example, if the User component is called from react-router,

export default function User({ match }) {
  const id = match.params.id;
  const user = useSelector(selUser, id);

This makes it possible for

• the selector to be unaware of where the component gets its data from
• the component to be unaware of how the store is structured.

Some of the arguments might be derived from the component properties, others might come from other sources like a user authorization context retrieved via useContext. Whatever the source, the interface in between the component and the selector is clean and clear.

The hook is internally responsible to provide useMemo with the dependencies it needs, which are none other than the selector itself and its arguments. There is no need to explain to the developer using useSelector what the deps argument is, which requires knowledge of the inner workings of the hook itself.

[josepot]

Hi @Satyam !

Sorry but these are not selectors:

export const selUsers = () => state => state.users.data;
export const selUser = id => state => state.users.data[id];

Those are selector creators (or factory selectors), not selectors. Meaning that they are functions that return selectors.

Good news, though, is that if you want to have a hook that works like that, you could easily build it yourself using the normal useSelector:

const useFactorySelector = (factorySelector, ...dependencies) => {
  const selector = useMemo(() => factorySelector(...dependencies), dependencies);
  return useSelector(selector, dependencies);
}

Therefore, I think that the current useSelector is a better primitive than the one that you are suggesting... I wouldn't be able to use the one that you are suggesting, because I don't like factory selectors.

[DuncanWalter]

As a side note, it is possible to use operator short circuiting to avoid instantiating new functions to be passed into useEffect within these redux hooks. I've heard people debate in the past about whether it is ok to instantiate functions within render calls as opposed to during component instantiation. Hooks fall into the render category, and should avoid heap if possible according to some. It's not super pretty, but these hooks can be optimized to run without any heap allocation on most renders. It might not be worth it depending on desired tradeoffs etc:

const noop: never = (() => { throw 'Should never be called' }) as any

// inside the useSelector hook
const effectWillBeCalled: boolean = someSmallOverhead()

useEffect(effectWillBeCalled ? () => { /*the usual logic*/ }) : noop)

Here's the implementation I've been tinkering with. Note that this is built on a state store that keeps track of multiple slices instead of one store and I'm not supporting swapping out selectors, so some of it isn't applicable: https://github.com/DuncanWalter/spider-web/blob/master/packages/spider-hook/src/useSelector.ts#L58

[Satyam]

@josepot Indeed, you are right, they are not selectors but selector factories, however, if I do it the way you suggest (which someone (I think @MrWolfZ) already pointed out), the useMemo inside useSelector is wasted as it would repeat the check on the dependencies and memoize (or not) what is already memoized. An optimization that would prove expensive.

I simply don't like an external API to be conditioned by an internal optimization I should not care about at all which, if misused as it will eventually be, can lead to bugs. Early optimization is not good, it is worst if it conditions the external API. I just mean to keep that optimization transparent to the developer and keep different concerns, component and store, separate.

An alternative would be to drop the useMemo of the selector from inside useSelector and let the developer memoize the selector if it proves necessary. Without that useMemo the deps argument would disappear and the API for useSelector would also be clear and your useFactorySelector function would be a great idea and would not waste time memoizing twice.

[cmrigney]

I have a question regarding patterns of use for the new hooks. Namely, that since the Container/Presenter pattern is no longer used with hooks (or recommended), what is the recommended pattern for using these hooks? Given this thread is not the appropriate place to start this discussion, should I post a new issue, ask in Reactiflux, or ask on Stack Overflow? After some Googling, I don't see any recommended pattern (which makes sense, since this feature is in alpha). So this may require some green discussion?

[markerikson]

@cmrigney : I'm not sure there's any specific "patterns", really.

You should feel free to call these hooks directly in whatever components you want. If you want to call them directly in components that are actually showing UI, go ahead. If you want continue to do a split between "container" and "presentational" components and only call them in containers, go ahead.

Really the same as all the rest of the hooks out there.

[SeanRoberts]

@cmrigney Kent C. Dodds did a good video answer about this after I asked him on his AMA, here's the link: https://www.youtube.com/watch?v=l6GTpKLWllQ&list=PLV5CVI1eNcJgCrPH_e6d57KRUTiDZgs0u

[joshjg]

@Satyam I think your use case is already better served by defining a custom hook:

const useTodoById = (id) => useSelector((state) => state.todos[id], [id]);

[MrWolfZ]

Btw, just to make this clear again, the deps of useSelector are only required if you need the selector to be referentially stable, e.g. if you are using a memoizing selector. If you just do a simple inline select like useSelector(state => state.todos[id]) the deps do absolutely nothing.

[janhesters]

@cmrigney I would suggest still going with the display / container pattern, because it makes your code more easily testable.

[gnoff]

Just chiming in here to say...

reselect's pattern of passing props implicitly to dependent selectors is problematic for a host of reasons, not only memoization breaking and makes little sense in the context of hooks where closure captured values are both idiomatic and don't allow for leaking selector props structures into components. They don't allow for safe composition of selectors. useSelector should not allow for using props in this way and I don't think supporting a useInstanceSelector would set users on the right path as their app and states continue to grow in complexity

separately, I agree useRedux is unnecessary (and implementable in user-land if desired). it shouldn't be part of core API since it can just as easily be composed out of the public API and shouldn't be encouraged IMO

lastly, what are the arguments for useActions when useDispatch exists? typing-wise it is just harder and it puts more load on hooks memoization to keep around this memoized state. I'm not necessarily arguing the api shouldn't exist, but it feels like useDispatch is just preferable in most ways

[josepot]

reselect's pattern of passing props implicitly to dependent selectors is problematic for a host of reasons, not only memoization breaking and makes little sense in the context of hooks where closure captured values are both idiomatic and don't allow for leaking selector props structures into components. They don't allow for safe composition of selectors

Hi @gnoff have you seen my proposal for Reselect v5? Also, have you seen Redux-Views? :slightly_smiling_face:

[markerikson]

@gnoff : useActions() exists both as a migration path away from connect(), and also for the same reason that the mapDispatch arg exists in the first place - to allow users to easily wrap up the process of dispatching a given action, particularly so that callback functions can be passed down to child components, without having to have access to dispatch at the call site. It's an incredibly common thing to do, so it makes sense to provide a hook for it. If you don't want to do that, that's why useDispatch() exists :)

[threehams]

useActions isn't much harder to type than dispatch(). In a few months (when we can bump the minimum TypeScript version to 3.4) it'll be easier still. Flow is another story but, well

The hardest part is that middleware affects the return value of bound actions based on the middleware that's applied. That's a series of conditional types and overloads. This is just a Redux thing, though, and we're working on getting the middleware libraries to work together.

Overall, nothing about the new hooks is difficult to type at all.

[MrWolfZ]

Just to provide more context on why useSelector is designed the way it is:

In my opinion in API design (and software engineering in general) there is the golden rule of "do the simplest possible thing that can work" (a.k.a KISS principle). In addition, for APIs, try to make as few assumptions as possible on how the API is going to be used.

Now for react-redux the simplest possible hook design would be useStoreState() but as we know, this has horrible performance issues (at least without proxy-based access detection). I believe that the current API proposal is the closest we can get to that. All the other designs I have seen in this thread make very concrete assumptions about how the hooks are used, often based on some third-party library's API (e.g. reselect et al). The current API does have a nod in the direction of these libraries by allowing you to specify 'deps', but it gives you absolute freedom in how you use this. Want to memoize the selector yourself? Fine, use useCallback, useMemo etc. and just don't provide deps. Want to have a curried factory selector? You can already do that. For all other proposed APIs it has been shown that you can easily create your own hook based on the current API in a few lines of code.

PS: On a side note, I have personally never used reselect and never felt the need to. I believe people tend to easily forget how fast pure JavaScript code is, even if it has to iterate over lots of elements etc. Even if I would use it, I would save it for the very few selectors where it is necessary while keeping the rest as inline lambdas. Most people I see using it use it universally. Now that may be how the majority of the userbase works with it, but we simply don't know, since we lack proper statistics. Since I was the one who implemented this API it is of course influenced by my personal views. Other people may have come up with other APIs that would also be perfectly fine, this is just how it turned out to be.

[MrWolfZ]

@gnoff let's just compare the usage with useDispatch and useActions in a scenario where we want to ensure callback stability since we pass the callbacks to child components.

useDispatch:

import { useCallback } from 'react'
import { useDispatch } from 'react-redux'
import * as actions from './actions'

export const Item = ({ id }) => {
  const dispatch = useDispatch()
  const updateName = useCallback(name => dispatch(actions.updateName(id, name)), [id])
  const deleteItem = useCallback(() => dispatch(actions.deleteItem(id)), [id])

  return (
    <div>
      <ItemName id={id} updateName={updateName} />
      <ItemDeleteButton deleteItem={deleteItem} />
    </div>
  )
}

useActions:

import { useActions } from 'react-redux'
import * as actions from './actions'

export const Item = ({ id }) => {
  const [
    updateName,
    deleteItem
  ] = useActions([
    name => actions.updateName(id, name),
    () => actions.deleteItem(id),
  ], [id])

  return (
    <div>
      <ItemName id={id} updateName={updateName} />
      <ItemDeleteButton deleteItem={deleteItem} />
    </div>
  )
}

I agree that the difference is not big, but I feel that useActions provides just enough in terms of conciseness and readability to be worth it. If you prefer the useDispatch approach, there is absolutely nothing wrong with it.

PS: For useActions if you don't want to bake the id into the callback, it becomes just so much simpler:

import { useActions } from 'react-redux'
import * as actions from './actions'

export const Item = ({ id }) => {
  const { updateName, deleteItem } = useActions(actions, [])

  return (
    <div>
      <ItemName id={id} updateName={updateName} />
      <ItemDeleteButton id={id} deleteItem={deleteItem} />
    </div>
  )
}

[gnoff]

Hi @gnoff have you seen my proposal for Reselect v5? Also, have you seen Redux-Views? 🙂

@josepot It looks like a nice improvement for the caching issue. other big issue is the idea in docs that component props should flow through the tiered selector calls which I think remains unchanged. you can technically just opt out by wrapping all sub selectors in a function which prepares an appropriate second argument to the selector but I don't believe I've ever seen that promoted as idiomatic.

@threehams

Flow is another story

:(

The hardest part is that middleware affects the return value of bound actions based on the middleware that's applied.

ah true, I don't really use these kinds of middleware so I was forgetting this happens and so I agree the typing issue is probably not harmed in any way and is already very challenging.

Just to provide more context on why useSelector is designed the way it is

@MrWolfZ yeah the current proposal is 👍 for useSelector. I was chiming in to make sure we didn't turn it into something more akin to current selector calls done in connect (props passed in automatically)

I feel that useActions provides just enough in terms of conciseness and readability to be worth it

@MrWolfZ / @markerikson After seeing it laid out like that the benefits are obvious, i suspected I was simply unaware of why useActions would be useful.

[markerikson]

I've just released 7.1.0-alpha.3, and this release removes the useRedux hook on the grounds that it doesn't provide any real benefit.

If you were calling useRedux() in your code, please replace it with separate calls to useSelector() and useActions().

[Torvin]

@MrWolfZ

PS: On a side note, I have personally never used reselect and never felt the need to. I believe people tend to easily forget how fast pure JavaScript code is, even if it has to iterate over lots of elements etc.

I mainly use reselect to guarantee props stability, i.e. to avoid unnecessary re-renders.

Even if I would use it, I would save it for the very few selectors where it is necessary while keeping the rest as inline lambdas. Most people I see using it use it universally.

This is due to the nature of reselect - its selectors depend on other selectors that should also be memoized. So you end up with all selectors that create derived state memoized (e.g. selectors that do store.foo.map(...))

[eduleite]

Is there a way to use this alpha with typescript? I want to try it on my project, but the actual @types/react-redux does not have the new useSelector and useActions. Checked npm and there is no alternative release for @types/react-redux.

[timdorr]

Nope, it hasn't been typed yet. The API may change, so it wouldn't be a good idea to build out types for it just yet.

[MrWolfZ]

@eduleite we are in fact maintaining a PR over at DefinitelyTyped with the latest hooks type definitions. You can extract those for the hooks and use it via module augmentation. Basically, just do what this sandbox does. You should be able to simply copy the react-redux-hooks.d.ts file from there into your project.

[eduleite]

@MrWolfZ thanks for your help!
Upgraded my project and converted my first component from connect to hooks. Working flawlessly here!

[josepot]

Hi all (but mainly @markerikson @MrWolfZ and @timdorr )!

I have finally had the chance to test this new API and I'm actually pretty worried about the "Zombie Children" issue.

First off, I would like to challenge the following text of the docs:

In version 7, that is implemented using a custom Subscription class internally in connect(), which forms a nested hierarchy. This ensures that connected components lower in the tree will only receive store update notifications once the nearest connected ancestor has been updated. However, this relies on each connect() instance overriding part of the internal React context, supplying its own unique Subscription instance to form that nesting, and rendering the with that new context value.

With hooks, there is no way to render a context provider, which means there's also no nested hierarchy of subscriptions. Because of this, the "stale props" and "zombie child" issues may potentially re-occur in an app that relies on using hooks instead of connect().

Unless I'm mistaken (and please forgive me if I am), if we were using React's Context in the way that it was intended to be used, then we would not have this problem. What I mean by that is that if I did this with React:

function Users() {
  const users = useContext(contextUsers);
  return users === null
    ? null
    : <UsersList />;
}

function UsersList() {
  const users = useContext(contextUsers);
  return users.map(u => <User id={u.id} key={u.id} />);
}

function User({id}) {
  const users = useContext(contextUsers);
  return <UserData {...users[id]} />;
}

I would never have to worry about the User component being evaluated with some data that it's not present in Context, because React already takes care of that for us, correct?

I understand why React-Redux decided not to rely on the "normal" usage of react-context in v7. If every time that a dispatch takes place, every single "connected component" gets a hold of the redux-state using React's context in order to evaluate if the "base component" needs to be updated... that implies paying an important cost on performance.

If I understood this correctly, this is why in v7 React-Redux decided to create a tree of internal subscriptions in order to ensure that we only force a render when it's completely necessary. That allows for those evaluations to happen outside of React's life-cycle, and it is a huge perf boost. I think that I understand that, and I also think that for the case of connect that makes a lot of sense.

However, I don't see the point of trying to prevent useSelector from triggering "unnecessary updates". I think that if useSelector was implemented (more or less) like this:

export default selector => {
  const state = useContext(reduxStateContext)
  return selector(state);
}

Then we wouldn't have the "zombie child" issue at all. Which BTW I've been suffering it in my tests, and it's very annoying. I understand that the problem with that ^^ implementation is all the "unnecessary updates" that would occur... That would imply that "the recommended way" of using useSelector would be in combination with useMemo, something more or less like this:

function Users() {
  const ids = useSelector(getUsers);
  return useMemo(() => <UsersList ids={ids} />, ids);
}

I also understand that implementation will never be as performant as:

const Users = connect(s => ({ids: getUsers(s)}), null)(UsersList);

Of course not. However, please notice that not even the current implementation of useSelector can't prevent updates that come from an ancestor update, while a "connected component", on the other hand, would.

What I mean is that even today, if we wanted to use React-Redux hooks and get a performance that is close to the performance that we would get when using connect, then you would have to use useSelector in combination with useMemo regardless (in order to at least get the "equivalent" performance boost of SCU).

The point that I'm trying to make is that hooks are lower-level primitives than HOCs are and they can't do the same things. So, IMO it's more important that what they do, they do it correctly. We should try to avoid perf optimizations that come at the cost of correctness.

Also, I really wonder whether that actually is a perf optimization anymore. Because if someone stops using connect and only uses useSelector, then all the subscriptions will be nested to the main subscription of the Provider. Which means that ALL those will be evaluated when a dispatch takes place. So, if there was a screen showing a list of users and as a result of a dispatch the users disappear from state, because we are about the render a completely different top-level component... After that dispatch, all the selectors that have to do with the now unexistent users will be evaluated again... And if they throw, then useSelector will swallow those errors :confused:. But what if the selectors are written in a way that they track unexpected errors before bubbling them up? Why should we have to re-write all selectors so that they can be evaluated with impossible states?

The point that I'm trying to make is that if performance really is important, then the user should consider either using connect or combining useSelector with useMemo. I really think that it is a lot more important that useSelector works correctly.

[markerikson]

@josepot : at work atm, so I can't give a long answer right now. Short answer:

We are _not_ going to bring back the v6 approach of propagating state updates via context any time soon. _If_ React ever vastly improves context performance and behavior, we _might_ consider it. Until then, the best performance comes from having external subscriptions that can run mapState or selectors, and decide if this component needs to update _before_ telling React to re-render. We're also not going to ship two different state propagation methods at the same time.

Trust me when I say I've spent way too much time thinking about this, and this is the right approach for now.

Please re-read #1177 for the recap and the details, as well as https://github.com/facebook/react/issues/14110 .

[MrWolfZ]

@josepot I am not sure if you have read all of #1179, but I think many of your concerns are addressed there. I will still try to respond to some of your points here:

• first of all, if you don't like the hooks API, nobody forces you to use it. You can just continue with connect or just create your own hook with the simple implementation you suggested
• the selector passed to useSelector should be a pure function, just as mapStateToProps for connect was, so selectors that track unexpected errors are using the API incorrectly (@markerikson I couldn't find anything about selector purity in the hooks docs; maybe we should add it?)
• I did quite a number of performance tests with different implementations for hooks (including your suggestion with useMemo) and the results show a huge difference between the current implementation and the simple one. In fact, the current implementation is just as fast as connect (sometimes it is slightly faster, sometimes slightly slower, but on average they are equivalent)
• you shouldn't worry too much about too many subscription callbacks being executed for each dispatch, since JavaScript (or rather modern browsers) are plenty fast enough that you won't even notice (especially if you use memoizing selectors)
• you are right that useSelector cannot prevent updates from ancestor components, but that's just how hooks are designed. However, you can easily get the same by just using the React.memo HOC. If that feels redundant to you, just continue using connect
• of course the current implementation is not perfect, but it is the best our collective minds could come up with that balances correctness, simplicity of the API, and performance

All of this being said, you mentioned you are suffering from zombie children in your tests. Can you provide a sandbox with such a scenario? I am very much interested in edge cases that I may have overlooked during implementation.

[markerikson]

@MrWolfZ : no, the hooks docs page doesn't talk about _what_ selectors are at all - the reader is assumed to already know that.

Having said that, both the current Redux _and_ React-Redux docs barely explain what selectors are to begin with, something I plan to address in the Redux core docs revamp.

[MrWolfZ]

I have created a PR with some adjustments to the docs that try to fill in some of the gaps that were brought up. I'd like to ask anyone interested to review it.

[josepot]

Hi @MrWolfZ and @markerikson ,

I'm sorry if my message came across as unfair criticism to the amazing work that you have been doing here. That was definitely not my intention. So, sorry if that was the case.

First and foremost, I really appreciate the work that it has been done and I'm very sorry for joining late to the party. I have now fully read #1177, #1179 and facebook/react#14110 and I do better understand how this decision was made.

I think that we all agree on something: which is that if React improved its context API in a way that made it possible to consume the context in a "conditional"/"selector" way, then it would be possible for React-Redux to use React's context without having to worry about performance issues. However, as the React team points out: the React Context API is currently not ready for handling the use cases of React-Redux. Perhaps one of the good things about having people trying to leverage React as a state-management library is that others will start experiencing perf-issues with the context API and that could motivate the React team to further improve it, who knows? :slightly_smiling_face:.

All that being said, I still want to respond to some of your responses:

@markerikson :

We're also not going to ship two different state propagation methods at the same time

I think that in a way we are already doing that. The fact that we get different behaviours when using connect vs using useSelector kinda proves that. Effectively, the way that useSelector propagates the changes is different from the way that connect does (the subscriptions won't be nested)... And it has to be, of course. The only thing that I'm saying is that since the way that useSelector propagates the state changes will always be different, perhaps it would make sense to sacrifice performance in pro of consistency/correctness.

On the other hand, one of the main constraints goals that were described on #1177 was:

Don't Re-Introduce "Zombie Child" Problems
Whatever solutions we come up with should avoid re-introducing this issue.

I understand why this implementation does a lot in order to mitigate the "Zombie Child" problems and I really appreciate that. However, IMO a variation of the "Zombie Child" problem will be re-introduced with the current implementation of useSelector.

@MrWolfZ :

first of all, if you don't like the hooks API, nobody forces you to use it.

I have said this before and I will say it again: I like the new hooks API. What I'm proposing is a different implementation for the useSelector hook, I'm not criticising the API. I'm worried about the implications of the current implementation. I think that those 2 things are quite different.

the selector passed to useSelector should be a pure function... so selectors that track unexpected errors are using the API incorrectly

There is no consensus of what a "pure function" is. I think that what's important is that the function itself is referential-transparent. I mean, someone could argue that a memoized selector is not a "pure function" because it keeps track of an external cache, therefore it's mutating something outside of the scope of the function. However, a memoized selector is referentially transparent and that IMO should be good enough.

The good thing about working with pure-functions is that they can be enhanced and composed, so that we can keep the side-effects at the boundaries of our systems. I think that it's perfectly reasonable to expect users to enhance their selectors in a way that's similar to this:

const logDevErrors = fn =>
  process.env.NODE_ENV === 'production'
    ? fn
    : (...args) => {
      try {
        return fn(...args);
      } catch(e) {
        console.error(`An unexpected error happened on the following function: ${fn.name}. The function's code is:`);
        console.error(fn.toString());
        console.error('The error message is:');
        console.error(e.message);
        console.error('The error stack is:');
        console.error(e.stack);
        throw e;
      }
    }

Sure, that enhancer would make the function to be "impure" for development... But logging and error-handling are both cross-cutting concerns and it makes a lot of sense to expect users to enhance their pure-selectors in a way that's similar to that, at least for development.

you shouldn't worry too much about too many subscription callbacks being executed for each dispatch

I'm not worried about that at all. I'm more worried about functions that are evaluated when IMO they shouldn't, not because of performance... I'm specially worried about useSelector swallowing errors of those selectors that shouldn't have been evaluated in the first place.

All of this being said, you mentioned you are suffering from zombie children in your tests. Can you provide a sandbox with such a scenario?

Absolutely. I will try to give you that in the next few days, for sure. And the example that I will provide will be inspired on a real-life App, totally!

Once again, I'm really thankful for all the hard work that it's been put into this and I'm sorry that I'm bringing these things up so late. I thought that this was the right place to give this feedback... At the end of the day, the only way to know for sure whether this implementation will be problematic or not is to ship it and see what happens when it's used in production. Good news is that as I already said before: I like this API, and one of the reasons is that it is not leaking implementation details. Which means that if in the future the "Zombie Child" problem is a real problem, then its implementation could be changed without that affecting the API... Or if in the future React provided with a better context API, of course.

Thanks again for getting back to me and for all the effort and thought that you've put into this.

[LoiKos]

Hi,

I'm working with react-redux hooks and i'm trying to figure out how i can use useActions hook inside useEffect. Eslint hooks rules not appear to be happy with it.

import { retrieveListIfNeeded } from '../../actions'

function ShipmentList(props) {
  const getShipment = useActions(() => retrieveListIfNeeded(), [])

 useEffect(() => getShipment(), [])

 return null
}

Should i added to dependencies ?

useEffect(() => getShipment(), [getShipment])

looks weird
Any other way to handle this ?

[MrLoh]

@LoiKos yes you have to specify your dependencies, that's simply how hooks work, Please read https://overreacted.io/a-complete-guide-to-useeffect/, this has absolutely nothing to do with redux.

[vikasgarghb]

Are you guys planning to add support for ErrorBoundaries?

[threehams]

Error boundaries are only available for class components so are irrelevant to hooks.

[markerikson]

@josepot : when I say "two different state propagation methods", I mean having components call store.subscribe() vs <Provider> doing this.setState({storeState : store.getState()}). We're not doing both.

The only difference between connect() and useSelector() in this respect is the nesting of the <ReactReduxContext.Providers overriding the Subscription instances in context. That's still the same mechanism either way.

I truly wish we had a "real" solution to the zombie child issues. We don't. We have a workaround. It's not ideal, but I'm willing to accept that as a tradeoff for getting out a public hooks API that works for the 98% use case.

[gaearon]

I'm seeing a lot of feedback that putting all "action creators" dependencies into useEffect, useCallback, etc dependencies lists is annoying. So I want to push stronger that I think the useActions() Hook is unnecessary, and useDispatch() Hook is more idiomatic in the Hooks world. See a more detailed comment in https://github.com/facebook/create-react-app/issues/6880#issuecomment-488158024.

I know I've pushed for this "binding action creators" pattern in the past. I've kind of regretted the amount of "clever shortcuts" we've added to Redux because people get overly fixated on these one-line "shorter" changes and miss the bigger picture. It's also harder to see how the data flows.

With Hooks especially, this object serves no real purpose because you immediately unpack it. There's even no component layer in between that could arguably be isolated. So useActions is just worst of both styles of APIs.

When you useDispatch(), your dependency list actually reflects your real dependencies. Because all action creators become static and move outside of component scope. Which, if you think about it — is what they really are. They're external dependencies and don't originate from a component or even from top-level Redux store in any way. There's no reason except past habits for why there should be a Hook that bind them. You don't useFunction(sum, 2, 2) to obtain a boundSum and then call boundSum. You just call sum(2, 2). This is the same.

[timdorr]

I'm in agreement with Dan on this one. Let's get rid of useActions now, while we're still in alpha. It's an anti-pattern and made more sense with connect than it does with Hooks.

[gaearon]

I'd also add that if you _really_ want to bind them in bulk for some reason, you always can import bindActionCreators from Redux that does exactly that. It's a one-liner.

[markerikson]

The primary arguments for action creators have always been around not having to have access to dispatch() at the call site (like a button a couple layers down), not having to hand-write action types inline, and preparing an action object appropriately (shaping arguments into a payload, generating IDs, etc).

Do you feel those arguments are no longer relevant here?

[gaearon]

If that’s your use case then bindActionCreators already solves it. Previously you couldn’t use it without changing your own props to include dispatch. Which was awkward. But now you can read it in the component body with useDispatch. So I don’t think it’s valid.

In practice I also think the argument with “separating” those layers doesn’t hold up that well anyway. It’s inconvenient and adds up the prop plumbing Redux users try to avoid. I’d suggest just useDispatch wherever you need it, and then only go to callbacks at low level like a Button which is unlikely to have complex effects etc.

[gaearon]

Additionally with a type system you don’t actually need action creators at all. At least not simple ones. They’re just empty cruft. But that’s a battle for another day.

[markerikson]

Okay, let's see if we can get some further feedback and opinions on this.

Per the comment above, Dan is proposing that we get rid of useActions and just ship useDispatch.

I've put up a Twitter poll asking if we should drop useActions.

Here's the tradeoffs as I see them:

• :+1: Arguments for keeping useActions:
  
  
  
  • Easier migration from connect
  
  
  • Standard approach for passing down action creators to children
  
  
  • Folks will probably want this anyway
  
  
  • Dropping it would force people to do a lot of "hand-binding"
  
  
  • I've always cringed at the function form of mapDispatch because of the repetition involved (one of the common "boilerplate" issues that I see), and only having useDispatch() seems like it would be basically the same issue.
  
  
• 👎 Arguments for dropping useActions :
  
  
  
  • Fewer issues with hooks deps arrays and the lint rule
  
  
  • Less conceptual overhead, more obvious what's happening
  
  
  • No name clash / hoisting issues with trying to destructure action creators from the useActions() result
  
  
  • We can still add it later as a minor/patch if we want
  
  
  • Dan said so :)
  
  

In addition to the Twitter poll, feel free to leave a :+1: if you want us to keep useActions, and a 👎 if you want us to remove useActions.

I personally would still want to keep it, but perhaps emphasize in the docs to prefer useDispatch() as your first choice.

Another option might be to remove it, but have a copy-pasteable useActions() implementation in a "Hooks Usage Guide" docs page.

[threehams]

Getting rid of it solves a big typing headache with middleware. Libraries can (and already do) overload bindActionCreators.

[josepot]

I don't feel strongly about whether useActions should be kept/removed, but I would like to challenge a bit the main argument for dropping it:

Fewer issues with hooks deps arrays and the lint rule

I actually don't know about this. For instance, the example that @LoiKos mentioned before. That could be implemented like this:

function ShipmentList(props) {
  const [getShipment] = useActions([retrieveListIfNeeded], []);
  useEffect(getShipment, []);
  return null;
}

And I _think_ that eslint wouldn't complain about that. Without useActions, it would have to be implemented like this instead:

function ShipmentList(props) {
  const dispatch = useDispatch();
  useEffect(() => dispatch(retrieveListIfNeeded()), [dispatch]);
  return null;
}

I personally find the latter a bit more imperative. Although, it is true that the difference is pretty superficial.

Personally, I don't use redux-thunk, I use redux-saga and I only dispatch events. In my case, I wouldn't dispatch more than one action in the same useEffect (*1)... I could argue that's almost always better not to do that at all (even if you use redux-thunk), but that's a different battle. Sorry.

---

*1 There could be a saga observing the action that was dispatched from the useEffect, and in some instances that action could trigger the dispatch of other actions depending on how the side-effects are being orchestrated. But in the useEffect I would just dispatch an action that describes what's happening... But that's just the way that I do Redux :see_no_evil: Although, I do think that even if you use thunks, perhaps it would make sense to have a thunk that handles all that stuff, rather than dispatching different things in the useEffect.

[hedgerh]

Fewer issues with hooks deps arrays and the lint rule

To be clear, the only reason action creators need to be passed as deps is just so a linting rule won't complain?

[markerikson]

@hedgerh : that seems to be the case, yes.

Now, addressing the somewhat larger question:

The Redux bindActionCreators utility basically only exists so it can be called as part of mapDispatch, either manually or when using the "object shorthand" form for connect.

As you can see in the source for bindActionCreators, all it actually does is:

• Check to see if the first param is a function or an object of functions
• Wrap all functions in (...args) => dispatch(actionCreator(...args)), effectively

So, folks could certainly call bindActionCreators in their own function components if they want, but if you're not dealing with an object full of action creators, there's actually not much of a point to using bindActionCreators. I mean, you can, but you might as well just do it "by hand" instead:

const dispatch = useDispatch();

const addTodoAction = useCallback(
    (text) => dispatch(addTodo(text)), 
    [dispatch]
)

Overall, I have to say I'm actually rather surprised by the poll results so far. The Twitter poll is at 17% "keep", 83% "drop" (120 votes), and the comment reactions are at +3 / -20. Could just be folks voting in the poll and clicking through to leave a reaction too, but consistent percentages.

Well. Tell you what. I'll push out alpha.4 that drops useActions, and I'll update the docs to say "useActions has been removed, but here's a copy-pastable version if you still want it".

[markerikson]

Awright, I've pushed out v7.1.0-alpha.4, which removes useActions.

I've also updated the hooks docs page to show it's been removed, and add a copy-pastable implementation.

[MrWolfZ]

Hmm, I kinda slept through this discussion sadly. I am still heavily in favor of useActions. Here's why:

• the linting argument is just strange, since a) it does still exist with useDispatch since you'll have to include dispatch in the deps always as well (even though we know it will never change as long as you use a single store), and b) the rule should instead just be made smarter, e.g. "any value returned by a hook with empty deps is assumed to be stable and can be ignored for deps" (I have no idea if this will work or if it is too short sighted, but I'd rather improve the linting rule than have libraries make concessions in their API design to the linting rule not being smart enough)

• yes, useActions is mostly a wrapper around bindActionCreators, but one that is aware of the rules of hooks and makes it easy to achieve stability of references for passing callbacks as props. Just take the example above from @markerikson:

  const addTodo = useActions(todoActions.addTodo, [])
  
  // versus
  
  const dispatch = useDispatch();
  const addTodoAction = useCallback(
      (text) => dispatch(addTodo(text)), 
      [dispatch]
  )
  

  and

  const { addTodo, updateTodo } = useActions(todoActions, [])
  
  // versus
  
  const dispatch = useDispatch()
  const { addTodo, updateTodo } = useMemo(
      () => bindActionCreators(todoActions, dispatch), 
      [dispatch]
  )
  

  I would argue that the useActions versions are much more readable (and yes, less to type, but that's not my main concern)

• one of the listed contra arguments by Mark is "Less conceptual overhead, more obvious what's happening". If that is the case, maybe the naming needs to be improved instead? Maybe call it useBoundActionCreators?
• another contra argument is "No name clash / hoisting issues with trying to destructure action creators from the useActions() result", which is just not true as you can see from the first dispatch example above which has the exact same problem (as long as you don't just use dispatch in anonymous callbacks as event handlers)

• I know that static typing is not the main concern here, but for a single action useActions is much more convenient than dispatch (in the case where you want callback stability). That is because ...args is tricky to type:

  const todoActions = {
    addTodo(title: string, description: string, expiresAtEpoch: number): {
      return {
        type: 'ADD_TODO',
        title,
        description,
        expiresAtEpoch,
      }
    }
  }
  
  // this will give addTodo the correct signature
  const addTodo = useActions(todoActions.addTodo, [])
  
  // versus
  
  const dispatch = useDispatch()
  
  // this won't work since ...args can not be inferred
  const addTodoAction = useCallback(
      (...args) => dispatch(addTodo(...args)), 
      [dispatch]
  )
  
  // so you have to add the types manually
  const addTodoAction2 = useCallback(
      (title: string, description: string, expiresAtEpoch: number) => dispatch(addTodo(title, description, expiresAt)), 
      [dispatch]
  )
  
  // or use `bindActionsCreators`
  const addTodoAction3 = useMemo(
      () => bindActionCreators(todoActions.addTodo, dispatch),
      [dispatch]
  )
  

  I'd argue that all the dispatch versions above are really inconvenient and actually make it harder to understand what's happening

• let's just agree that comment up- and downvotes as well as twitter polls are far from representative and it's dangerous to base API decisions on this

[markerikson]

@MrWolfZ :

let's just agree that comment up- and downvotes as well as twitter polls are far from representative and it's dangerous to base API decisions on this

Agreed, but:

• It's a strong suggestion from Dan, and Tim is also in favor
• initial feedback was surpisingly in favor as well
• It's an alpha, this is the time to make changes
• We could ship without it initially and then add it back in as a minor / patch if we want.

It's an experiment. Let's see what happens.

[MrWolfZ]

@markerikson Yeah, let's have some people switch their alpha tests from useActions to useDispatch and see what their feedback is.

[janhesters]

@MrWolfZ

let's just agree that comment up- and downvotes as well as twitter polls are far from representative

I'm curious, what makes you assume that?

Aren't the people reading Mark's tweets and this thread the most credible when it comes to Redux? If you know who Mark is and you read this thread, you probably know Redux well (and Hooks, too).

The people I talk to on meet-ups that say "useCallback does what?" generally don't know Mark, Sophie or other around the React world (with the exception of Dan). And those who know what they are talking about generally recognize Mark's (and other people's) names. I think there is a correlation making the poll at least somewhat representative.

[janhesters]

Just to give an UX example: I migrated from using HOC's to Redux Alpha with Hooks over the last few days.

Using the HOC I had several bound action creators in my deps for useEffect and useMemo. Migrating them with useActions was easy. But the "huge" arrays where a little annoying.

I only thought about using useDispatch after reading Dan's and Mark's conversation on Twitter and I gotta say the code using useDispatch looks cleaner and more declarative. I like API's that force you to write better code, which I think omitting useActions does. It's one of those "once you understand it" things.

The most valid pro useActions argument might be @MrWolfZ typing argument for TS.

[MrWolfZ]

@janhesters

Aren't the people reading Mark's tweets and this thread the most credible when it comes to Redux? If you know who Mark is and you read this thread, you probably know Redux well (and Hooks, too).

The people I talk to on meet-ups that say "useCallback does what?" generally [...]

No, they are exactly not. Those "useCallback does what?" people are probably the majority of the react and redux userbase. Therefore their opinion regarding the API would be the most representative (even if not the most informed) opinion. Code that uses dispatch all over the place might be very obvious and readable to people very familiar with redux, but for the others I think that a simple function call is more intuitive (but due to the response bias in polls I cannot back that belief up with proper statistics).

[hedgerh]

I'm pretty torn on this.

• On one hand, I prefer using dispatch directly when using connect, and see why some people prefer using dispatch directly instead of useActions, but the useDispatch usage is looking pretty gnarly in @MrWolfZ's callback examples, particularly with Typescript. It doesn't seem to be reducing complexity there.

• I agree it's weird that a linting rule is influencing the API. I believe the rule insists that deps are passed in, even when you know the action creators will never change. This sounds like the linting rule is causing long dependency lists and runtime overhead, rather than useActions, no? Apparently the only workaround is to disable linting on the line.

If that is the case, maybe the naming needs to be improved instead? Maybe call it useBoundActionCreators?

• I don't see the point. It's not like useActions creates some insane mental overhead. "Less conceptual overhead, more obvious what's happening" Does this really help anyone? The pattern has been around for years via mapDispatch and we've managed to get by. It doesn't seem like the useActions abstraction is fundamentally broken to the point that it's better to accept a worse developer experience.

• I know it's an experiment, but wanted to share my thoughts. I'm going to work on some samples to see what it's like working with useDispatch vs useActions.

[hedgerh]

No, they are exactly not. Those "useCallback does what?" people are probably the majority of the react and redux userbase.

👏 react-redux averages 2 million downloads a week. 200 people voted in the poll. When designing something, power users aren't usually the people you want to base your decisions off of. With all respect to Mark, Dan and Tim were the major tipping factors in this decision, with the polls being added confirmation.

(but due to the response bias in polls I cannot back that belief up with proper statistics)

With this being a bit of an experiment, I'm not sure how we'll get a good idea of whether it's a good idea or not pre-release. I don't expect the other 99% of developers out there to be trying out the alpha. Hmm..

[hedgerh]

Idea if this makes it to release: instead of sharing the code for useActions in the docs, release it as a tiny module and measure the downloads. That'd give us some real data.

[janhesters]

Those "useCallback does what?" people are probably the majority of the react and redux userbase. Therefore their opinion regarding the API would be the most representative (even if not the most informed) opinion.

Good point, now I see why you say the polls aren't represantative.

I think you can say two things:

1. The majority of experienced Redux developers seem to agree that useActions encourages lower quality code.
2. We don't know what the unexperienced majority thinks.

My question would now be, should we design an API that encourages better code, even though it might not be the easiest to learn? (In my opinion, yes, because useDispatch is still easy enough for even new users to understand. And this is a way of implicitly mentoring people about dependencies.)

200 people voted in the poll. When designing something, power users aren't usually the people you want to base your decisions off of.

You left out the reason. Why shouldn't power users design the API?

[MrWolfZ]

@janhesters

My question would now be, should we design an API that encourages better code, even though it might not be the easiest to learn? (In my opinion, yes, because useDispatch is still easy enough for even new users to understand. And this is a way of implicitly mentoring people about dependencies.)

To be honest, I still don't understand why explicit dispatch is better code. In what way?

[janhesters]

@MrWolfZ

I still don't understand why explicit dispatch is better code. In what way?

I hope this will not cause this thread to be a debate war about what is good code. Feel free to mark my comment as off-topic!

I feel like Dan already expressed it well:

When you useDispatch(), your dependency list actually reflects your real dependencies. Because all action creators become static and move outside of component scope. Which, if you think about it — is what they really are.

I think rather then helping less experienced developers it will confuse them more.

function Component() {
  const actions = useActions(importedActions)
  useEffect(() => {
    actions.increment()
    actions.toggle()
    fetch().then(() => {
      actions.decrement()
    })
    // New developer might be confused here, missing that dispatch is only relevant.
  }, [actions.increment, actions.toggle, actions.decrement]);
  // ...
}

versus

import { increment, toggle, decrement } from './actions';

function Component() {
  const dispatch = useDispatch();
  useEffect(() => {
    dispatch(increment())
    dispatch(toggle())
    fetch().then(() => {
      dispatch(decrement())
    })
    // New developer learns to correctly only put dispatch in the array.
  }, [dispatch]);
  // ...
}

I think there is a reason Dan wrote about useEffect. There is probably a lot of confusion still going on about dependencies.

In fact, here is a thread of me two weeks ago being confused by dependency arrays. The docs as they are currently with a detailed explanation about leaving out useActions forces you to have a critical look at dependencies.

[MrWolfZ]

@janhesters In the particular scenario you showed there is indeed not a big difference between the styles, but in the "pass callback as props" scenario I outlined it becomes much more pronounced.

That said, your example for useActions is incorrect. Since you don't pass a deps to useActions, the creators will be bound every time, resulting in new references, therefore the effect is executed on every render. In your example the correct code would be this:

function Component() {
  const actions = useActions(importedActions, [])
  useEffect(() => {
    actions.increment()
    actions.toggle()
    fetch().then(() => {
      actions.decrement()
    })
  }, [actions]); // alternatively pass in all used actions directly
  // ...
}

I would say this is just as explicit about the dependencies as using dispatch.

Lastly, the majority of users will ever only have a single store in their app, so neither dispatch nor the bound action creators will ever change, so leaving the deps array empty would be equivalent.

PS: your example made me notice a bug in my useActions implementation, since it does not add dispatch to the deps for useMemo

[adrien-gueret]

As a newbie developer concerning React hooks, I was first a bit confused with this decision to remove useActions hook. But after reading all the discussions and some articles, it makes a lot more sense... I guess.

I've tried to create a poc for testing this alpha. Since we don't have anymore useActions, I've tried to get my own useAction (handling only once action): I like the idea to let my component unaware of Redux, that's why I prefer to use useDispatch outside of it.
Here is my custom hook: https://github.com/adrien-gueret/test-redux-react-hooks/blob/master/modules/utils/hooks/useAction.js

It works well, but is it the righ way to use useDispatch? Or is it an anti-pattern?
I'm also not very sure about deps, so I put them anywhere I can ^^'

Not sure if my comment is useful or not, at least you can see what a developer new to hooks can try...

[richardcrng]

I've just been playing around at moving across some connected components over to the React-Redux hooks API, and wondered whether hooks somewhat like the following might be useful for migration purposes?

export function useMapState(mapState, deps = []) {
  const state = useSelector(state => state)
  return useMemo(() => mapState(state), [state, ...deps])
}

export function useMapDispatch(mapDispatch, deps = []) {
  const dispatch = useDispatch()
  return useMemo(() => mapDispatch(dispatch), [dispatch, ...deps])
}

(This seems to suit my own use case quite well - I like how I can just slot in my current mapStateToProps and mapDispatchToProps functions)

(EDIT: better to default deps to [], I think?)

[MrWolfZ]

@richardcrng I would suggest you read #1179, even though it is long. A lot of these things have been discussed.

A short feedback in any case:

• your useMapState and the existing useSelector are basically equivalent, except that your code suffers from major performance issues by forcing the component to re-render on every dispatch
• defaulting deps to [] is a bad idea since it can cause difficult to detect and analyze bugs if mapState or mapDispatch close over props without deps being provided
• neither of your hooks really just allows plug-and-play of existing functions since they don't provide props (which in the hooks world should be provided via closure anyways); you can of course just do useMapDispatch(dispatch => myMapDispatch(dispatch, props), [...]), but then the deps get tricky since something like ...Object.values(props) is not guaranteed to work

[richardcrng]

@MrWolfZ ah, thanks for taking the time to point me there! (I stumbled across this thread by accident a couple of hours ago so I have neglected to look into this in depth, apologies)

[nperichSYKES]

Just catching up on all this now, so apologies if this has been discussed to death, but if the vast majority of times you call an action is in useEffect or useCallback (right?) and the only dep for useEffect is dispatch, what about also exposing official hooks that wrap that little boilerplate like "useDispatchEffect" and "useDispatchCallback"?

function SomeComponent() {
    useDispatchEffect(dispatch => {
        dispatch(someAction())
        dispatch(anotherAction())
    }, [])

    return ....
}

Idk, just a thought. Maybe it would be cleaner to add a docs section on such utility hooks rather than expose it on react-redux itself...or do no such thing at all! But I'm just thinking if that's the default way to call actions it would be a couple less things to explain to people who are new to redux or don't use it every day (no dispatch as a dep, no "getting dispatch()", no remembering that/why it's different from internal react state dispatch()).

[3nvi]

I guess one of the downsides to having only a useDispatch() hook in the artillery, would be the fact that you now need an anonymous function to fire an action. If I pass that anonymous function to another component, then this component cannot benefit from render bail-out techniques (PureComponent, memo, etc.).

To avoid that, I have to wrap the anonymous function in a useCallback() hook in order to be referentially the same on every render. Thus, those 2 hooks go hand-in-hand now when performance is a key for your app. Kind of like that :

const dispatch = useDispatch();
const handleIncreaseCounter = useCallback(
    () => dispatch(increaseCounterAction()), 
    [dispatch]
);

I also find it interesting that the dispatch function is exposed and is the recommended approach, especially after years of trying to "conceal" it behind the connect() HOC. I would expect the hooks to kind of match the way connect() is implemented since new users now might be confused as to which action firing approach is the best.

[Yakimych]

@3nvi I find it interesting too. One of the reasons we never use dispatch in our components is to make them unaware of redux and allow us to have a "connected" and "raw" versions of the component, that could be used in both projects that use redux and projects that don't (yes, we have both).

@markerikson - I suppose this won't be possible with the hooks API at all, since now everything is mashed together in one function?

[LoiKos]

@MrLoh sorry for responding so late ! I already read dan blog post on overreacted ( really useful btw ).
Inside the blog post Dan says the best way is to include function inside useEffect and their it's not possible and I don't see how to use useCallback with it. Maybe lint rules is just too strict.

[3nvi]

@Yakimych Well you cannot escape the "unaware" part when hooks step into the game. I just find the approach kind of contradicting to the existing API.

[MrWolfZ]

@Yakimych what you are describing is similar to the container and presentational component pattern. You can still do that with hooks. Just have a container that selects the relevant pieces from the state and binds action creators to dispatch. Then pass all of this as props to your "raw" component that is unaware of redux.

[dusty]

I think one of the benefits of hooks is composing your own custom hooks. If one doesn't like to use dispatch inside their components, they can hide it inside a custom hook.

// actions.js
const saveTodo = todo => ({type: 'SAVE_TODO', payload: todo})

const useTodoActions = () => {
  const dispatch = useDispatch() 
  return {
    loadTodos: () => dispatch(loadTodos()),
    saveTodo: todo => dispatch(saveTodo(todo)),
  }
}

// todos-component.js
const TodosComponent = () => {
  const { loadTodos, saveTodo } = useTodoActions()
}

[dunika]

I agree that useActions is no good when you need to declare any hooks that depend on the returned actions object. It also feels a bit bulky in terms of what I feel a hook should be handling in regards to the react-redux library. For me the hooks should be as close to the metal as possible.

With that being said I do find dispatch on its own to be unnecessarily verbose when you want to dispatch an action creator that takes arguments.

  const dispatch = useDispatch();

  const action = (...args) => dispatch(actionCreator(...args));

  const actionTwo = useCallback((...args) => dispatch(actionCreatorTwo(...args)), [dispatch]);

  ...

  <Form
    onSubmit={(values) => dispatch(actionCreator(values))}
  />

I really don't want to have to think about having to pass arguments into my dispatched actions. I propose a new hook, useAction, that could be defined as follows:

  const useAction = (actionCreator, deps = []) => {
    const dispatch = useDispatch();
    return useCallback((...args) => {
      const action = actionCreator(...args);
      dispatch(action);
    }, [dispatch, ...deps]);
  };

  ...

  const action = useAction(actionCreator);

  ...

  const action = useAction(() => {
    type: 'ACTION',
    payload: prop
  }, [prop]);

I don't think its too much of an abstraction on top of useDispatch and it doesn't carry with it the as much dependency overhead from useActions. I also feel that it greatly improves the readability and reduces the things I need to think about; primarily the passing of arguments to my action creators and the use of useCallback #prematureOptimizationFTW

Cons

• Additional overhead of multiple calls to useDispatch in components with multiple actions
• All the other arguments against useActions still mostly apply
• Could easily be left up to the user to implement themselves

[MrWolfZ]

@dunika useActions already provides exactly the functionality you propose (plus some overloads for objects and arrays)

Also, to state this again: an empty deps array as default is a bad idea, since it makes it too easy to accidentally lie about your dependencies.

[Yakimych]

@MrWolfZ

Just have a container that selects the relevant pieces from the state and binds action creators to dispatch. Then pass all of this as props to your "raw" component that is unaware of redux.

Yes, it's possible, but it wouldn't make much sense to use the hooks API in this case, since with the old connect API we're talking about pretty much just adding an export to the raw version. With hooks, I would have to refactor much more. I assume the old connect API is not disappearing, right?

[markerikson]

@Yakimych : correct - nothing about connect() is changing whatsoever. The new hook APIs are purely addition to the existing APIs we deliver.

To be honest, in a lot of ways the point of hooks _is_ to remove that separation between "container" and "presentational" components. Some folks may like that, some may not. It's certainly true that you can deliberately hand-write separated components where the "container" extracts data using a hook and passes it to a "presentational" component... but at that point, you've basically reinvented connect() and might as well just use it directly :)

[Yakimych]

@markerikson

but at that point, you've basically reinvented connect() and might as well just use it directly :)

Yep, exactly my point. As for separation between container and presentational components - I don't really see it that way. For me it's more about separation of concerns and keeping the "React" and "Redux" parts clean and isolated, not whether components themselves are "container" or "presentational".

The same way as the "redux part" of the application doesn't know that React views are the consumer, the React components ideally shouldn't know that Redux is the "state provider". It's not purely theoretical either - we ran into real issues on an existing project when we had to painfully pull apart old components that were too redux-dependent (I think via redux-form), when we wanted to use them in non-redux projects.

[dunika]

@MrWolfZ

useActions already provides exactly the functionality you propose

Its funny because that why I originally came here! I was using useActions to bind single actions and noticed an error when I upgraded to the latest alpha. And while it does provide the same functionality it is conceptually different.

I think there is less ambiguity with something like useAction since it only handles the single case of binding one action. It feels like a happy medium between useDispatch and useActions.

Also, to state this again: an empty deps array as default is a bad idea, since it makes it too easy to accidentally lie about your dependencies.

I'm not really sure how this applies here as there is already an initial dependency of dispatch being passed into useCallback unless I am missing something?

[MrWolfZ]

@dunika the problem with empty deps can be seen in this example:

export const TodoComponent = ({ id, text }) => {
  // this will cause an issue with stale props if the `id` changes since the
  // action will not be re-bound due to the empty default `deps`
  const deleteTodo = useAction(() => ({ type: 'DELETE_TODO', id }))

  return (
    <>
      <span>{text}</span>
      <button onClick={deleteTodo}>Delete</button>
    </>
  )
}

The proper way of doing it would be to leave the deps undefined if not specified, and only add dispatch if deps are provided.

[dunika]

@MrWolfZ

Ah I see now thanks! So just for completeness and while I'm updating the code in my project I may as well throw it here as well.

  const useAction = (actionCreator, passedDeps) => {
    const dispatch = useDispatch();
    const deps = passedDeps ? [dispatch, ...passedDeps] : undefined;
    return useCallback((...args) => {
      const action = actionCreator(...args);
      dispatch(action);
    }, deps);
  };

[lukaszfiszer]

I spent some time playing with new hooks API, and one remark that I have is I don't really see the purpose of second, deps argument to useSelector hook:

1. the tests show that it doesn't play any role in preventing stale props issue

2. when using a memoizing selector like reselect you want to keep the same selector instance during the whole lifetime of a component - this can be easily done with useMemo. If one of the inputs comes from props then they become the second input of a selector. Example:

export const OtherTodoListItems = (props) => {
  const selectAllOtherTodos = useMemo(() => createSelector(
    state => state.todos,
    (_, props) => props.id
    todos, id => todos.filter(todo => todo.id !== id)
  ), []);
  const otherTodos = useSelector((state) => selectAllOtherTodos(state, props))
  return <div>{otherTodos.length}</div>
}

1. deps actually encourage writing selectors which are not pure, but depend on props from their closure. This makes them harder to test and extract from the component. See this example from PR #1267 by @MrWolfZ

export const OtherTodoListItems = ({ id }) => {
  const selectAllOtherTodos = createSelector(
    state => state.todos,
    // here "todos" come from input selector, but "id" from outer closure
    todos => todos.filter(todo => todo.id !== id)
  )
 const otherTodos = useSelector(selectAllOtherTodos, [id])
  return <div>{otherTodos.length}</div>
}

1. Memoized selector are an optimisation that most application probably don't need. I would argue that as a such it should not be directly exposed in react-redux API, which aims to be as minimal as possible.

[MrWolfZ]

I'd like to make one last effort to make a case for useActions. Just to be clear, I don't have a personal stake in this and I can live with only useDispatch just fine, but I feel the arguments for useActions outweigh the arguments against it. In fact, the only real arguments against it that have been made in this issue are "less problems with the linting rule" (which just means you sometimes have to put actions into the dependencies list even if you know they won't change - which btw is also the case with useReducers dispatch) and the statement by @gaearon that it is more idiomatic to use useDispatch directly (based on this example which as I have shown here is wrong since you can just add only actions itself into the deps, just as you would have with dispatch).

So, here are my core arguments for useActions:

• it is one of the current modes of operation for mapDispatchToProps to auto-bind action creators, therefore leaving it out can make the migration path more difficult (it is even the recommended approach to use this mode)
• having useActions does not prevent you from only using useDispatch if you feel that is more idiomatic; the question is this: is react-redux an opinionated library that wants to push a certain way to write code to its users or is it an unopinionated library that offers you a choice of different APIs to use that best fits your code and style?
• it makes it much easier to pass stable callbacks to child components without needing to use useCallback; yes, @gaearon may not prefer passing callbacks as props and rather just dispatch actions directly from each component but some users will still want to do this, for example to separate redux-aware components from redux-unaware ones
• without useActions properly statically typing the actions can become tricky (see the second to last bullet point here); sure, if you never need to wrap your action in useCallback and just always dispatch directly this is not an issue, but again, this library has millions of users and so far the spirit in its API design seems to have been to provide as many options as possible (I would argue sometimes even too much, but that's a discussion for a different day)

I understand that it is often better to start with a minimal API and expand on that later on. useActions can always be added later if required, but if we anticipate it is going to be added anyways, why make users go through the troubles of migrating to it later on? Also, some people in this thread have already rolled their own versions of the hook, which sometimes are not completely correct. Do you really want lots of different potentially buggy implementations out there?

Also, if you decide to leave it out, I wouldn't put it into the docs. Sure, it may look simple enough and probably won't change much, but through the recent discussions we have already noticed a bug in it. If further bugs are discovered and fixed in the docs, most users will never see that fix and silently suffer from the bug. If you feel it needs to be there, why not just ship it with the code?

[MrWolfZ]

@lukaszfiszer the point of the deps is not to prevent stale props. That said, I completely agree with your examples. When I added deps initially to useSelector I thought it would make using memoizing selectors much easier, but while adjusting the docs and looking at your examples, it becomes clear that just explicitly memoizing the selector with useMemo may be the better way to go. You can of course use a factory selector instead of directly depending on props via closure (see example below), but this API seems difficult to use correctly. If we drop the deps we just need to make sure to have a proper example of how to use useSelector with a memoizing selector in the docs.

Here the example with a factory selector:

// selectors.js
export const makeSelectAllOtherTodos = id => createSelector(
  state => state.todos,
  todos => todos.filter(todo => todo.id !== id)
)

// component.js
export const OtherTodoListItems = ({ id }) => {
 const otherTodos = useSelector(makeSelectAllOtherTodos(id), [id])
  return <div>{otherTodos.length}</div>
}

[lukaszfiszer]

@MrWolfZ I'm not an reselect expert, but looking at the docs I think the equivalent of your example without deps would be

// selectors.js
export const makeSelectAllOtherTodos = () => createSelector(
  (state) => state.todos,
  (state, props) => props.id,
  (todos, id) => todos.filter(todo => todo.id !== id)
)

// component.js
export const OtherTodoListItems = (props) => {
  const selectAllOtherTodos = useMemo(() => makeSelectAllOtherTodos(), [])
  const otherTodos = useSelector((state) => selectAllOtherTodos(state, props))
  return <div>{otherTodos.length}</div>
}

If there is just one instance of <OtherTodoListItems> (common case for "container" components) then the selector factory and useMemo are not needed at all:

// selectors.js
export const selectAllOtherTodos = createSelector(
  (state) => state.todos,
  (state, props) => props.id,
  (todos, id) => todos.filter(todo => todo.id !== id)
)

// component.js
export const OtherTodoListItems = (props) => {
  const otherTodos = useSelector((state) => selectAllOtherTodos(state, props))
  return <div>{otherTodos.length}</div>
}

[MrWolfZ]

Yeah, I think putting those two examples in the docs should be enough to replace deps. What do others think?

[josepot]

What do others think?

I agree, it makes sense to remove the deps from useSelector. However, I'm not sure about those 2 examples.

I would write the first example like this, instead:

// selectors.js
export const makeSelectAllOtherTodos = (id) => createSelector(
  (state) => state.todos,
  (todos) => todos.filter(todo => todo.id !== id)
)

// component.js
export const OtherTodoListItems = ({id}) => {
  const selectAllOtherTodos = useMemo(() => makeSelectAllOtherTodos(id), [id])
  const otherTodos = useSelector(selectAllOtherTodos)
  return <div>{otherTodos.length}</div>
}

Also, I think that the second example is very confusing. If there are not going to exist many different instances of OtherTodoListItems, then I think that the selector shouldn't depend on props. Because in reality, if that was the case, that id could come directly from the state... And if it really comes from outside the state, then you would be better off just using the same code as in the first example.

[Yakimych]

Ok, just tested converting my TypeScript project to using the hooks API with alpha-4 - pretty much straightforward. One thing I run into is useDispatch for dispatching a thunk.

Previously I typed dispatch in mapDispatchToProps as ThunkDispatch<MyState, void, MyAction>:

const mapDispatchToProps = (
  dispatch: ThunkDispatch<MyState, void, MyAction>
) => ({
  normalProp: () => dispatch(actions.normalActionCreator()),
  thunkProp: () => dispatch(thunkActionCreator())
});

My best attempt so far is: const dispatch = useDispatch<ThunkAction<void, MyState, void, MyAction>>();, but this doesn't work: Property 'type' is missing in type 'ThunkAction<void, MyState, void, EmptyAction<"API_CALL_STARTED"> | PayloadAction<"API_CALL_ERROR", string> ...' but required in type 'Action<any>'

@MrWolfZ, other TS gurus - any idea how to do this?

[lukaszfiszer]

@josepot

1) Your first example looks neat, but I does not follow reselect conventions. In almost all examples from their documentation props are passed as second argument to the selector - see this chapter
and and this one. Most people probably have selectors written in this style already. Keeping it will make the migration to hooks easier, as folks won't have to modify existing selectors.

2) Totally agree on the second example. However, I think that docs should show a "minimal" example of a memoized selector that accepts state, without props. For example:

// selectors.js
import { createSelector } from 'reselect'

export const selectPendingTodos = createSelector(
  (state) => state.todos,
  (todos) => todos.filter(todo => todo.completed !== true)
)

```jsx
// component.js
import {selectPendingTodos} from "./selectors";

export const PendingTodoListItems = () => {
const pendingTodos = useSelector(selectPendingTodos)
return

{pendingTodos.length}

}

[Satyam]

I agree with the above. Memoizing is not part of the core functionality expected of useSelector. It can be handy, but other packages do or may provide memoizing by themselves, unnecessarily duplicating what is no more than an optimization which may not be warranted at all. Besides, the deps argument causes too much trouble requiring far more explaining in the documentation than it is worth. Having useMemo removed from within useSelector does not prevent it from being used outside. And, if so, if there is any issue with improper deps it is clear where the problem is.

With useActions dropped in favor of useDispatch there are no further deps in any of the arguments, to any of the hooks, saving lots of trouble, both in the documentation and in further support issues which will eventually arise.

[josepot]

@lukaszfiszer

Your first example looks neat, but I does not follow reselect conventions. In almost all examples from their documentation props are passed as second argument to the selector...

Sure, but those examples were written like that because connect accepted both factory-selectors and selectors that accepted props as a second argument. Now that we have hooks, and given the fact useSelector doesn't accept either factory selectors or selectors with props, it makes more sense to update those examples :slightly_smiling_face:. Although, I'm quite optimistic about the chances of this proposal of Reselect v5 being accepted and if that happens, then Reselect won't be proposing the usage of factory-selectors any longer :smile:.

Totally agree on the second example. However, I think that docs should show a "minimal" example of a memoized selector that accepts state, without props. For example

:100:

[DuncanWalter]

@Yakimych In my experience with typing redux with thunks, sometimes it is best to alter the types exported by the redux module by using a module declaration that has additional overloads in definitions for dispatch.

IIRC you can pull something along the lines of:

// types.d.ts
declare module 'redux' {
    export interface Dispatch {
        (action: Action): void
        <R>(thunk: ThunkAction<R>): R
    }
}

This means that all types which reference redux's Dispatch type are using your new and improved type, and you are free to add more overloads if more middleware gets added.

[josepot]

Hi @lukaszfiszer and @MrWolfZ,

I've opened a PR (#1272) for removing the dependencies of useSelector. Please let me know what you think about it and especially about the examples that I'm suggesting for the docs. (Maybe we can continue the conversation about what the best examples should be in that PR)

Thanks!

[EvHaus]

With the removal of useActions I'm having trouble migrating to useDispatch. I'm using redux-thunk it and it seems like I can't just call dispatch on thunk functions. This doesn't work:

const dispatch = useDispatch();

dispatch(() => {
    return (dispatch, getState) => {
        dispatch({type: 'ACTION'});
    })
})

What is the right migration path here? Is it to use bindActionCreators?

[josepot]

Hi @EvHaus !

I'm not an expert on redux-thunk, but I think that what you wanted to do is something more or less like this:

const yourThunkCreator = type => (dispatch, getState) => dispatch({type});

const YourComponent = () => {
  const dispatch = useDispatch();
  useEffect(() => dispatch(yourThunkCreator('ACTION')), [dispatch]);
  // more code here...
};

The main thing is that when you dispatch a "thunk" you have to dispatch a function that takes the dispatch and the getState as its parameters. In your code you are dispatching a function that returns the thunk, rather than dispatching the thunk itself.

[MrWolfZ]

@EvHaus I think the correct version would be without the outer lambda, i.e.

dispatch((dispatch, getState) => dispatch({type: 'SOME_ACTION'}))

However, usually you wouldn't want to declare thunks inline, but in an action creator.

If you want to declare the action inside the component you could instead just use an effect and get dispatch via useDispatch and getState via useStore.

[EvHaus]

Ah, of course. That was it:

// My original bad code
dispatch(() => {
    return (dispatch, getState) => {
        dispatch({type: 'ACTION'});
    })
})

// The correct solution
dispatch((dispatch, getState) => {
     dispatch({type: 'ACTION'});
}))

It's been so long since I had to manually bind actions that I stumbled over some redux basics. Thank you both.

[le0nik]

I agree with @ricokahler and @josepot that shallow comparison of returned part of the state in useSelector feels unnecessary.

I think it should be changed to Object.is as default because

1. Redux state should be treated as an immutable storage, so any change inside the object should change object's reference too. Comparing internal properties of user's objects doesn't make sense. It would be different if selector acted like this.setState({}) and was forced to return a wrapper object.
2. With shallow comparison in case of returning a container like a large object or array from the selector user is forced to accept the performance penalty when this container's reference changes unless they choose to wrap the return value themselves, which is very subtle and unexpected. Consider this example:

// state.items is a large array
// suppose we've just appended one item to it, so it's reference has changed
// compare 2 approaches
const items = useSelector(state => state.items); // [1]
const { items } = useSelector(state => ({ items: state.items })); // [2]

[1] all elements(potentially millions) of items array would be compared
[2] only items reference would be compared, which is enough because of immutable nature of state in redux
This wouldn't be a problem with Object.is.

[Yakimych]

@DuncanWalter - I kind of got it to work for my use-case by tweaking the definition of Dispatch from @MrWolfZ codesandbox:

export function useDispatch<
  A extends Action | ThunkAction<R, S, E, A> = any
>(): Dispatch<A>;

But I am not sure how this is going to work when the "official" typings are released.

Is defining own types really the only way to go here? I suppose the problem is essentially that there is no way to know at compile time what middleware is being used?

[MrWolfZ]

@le0nik the current design is a trade-off. It makes what we consider to be a fairly common pattern of selecting pieces of the state into an object easy and performant. Sure, if you select an array or object with millions of members, you might get into issues, but a) is this really a common use case, and b) if you have this kind of scenario, you can easily work around this like you showed in [2].

@Yakimych the PR for the types also contains tests for dispatching thunks and they should work just fine without any adjustments to the signature of useDispatch (granted, that's because it defaults to just any, but since any middleware can potentially accept any value, that's just how it is). I think there were also some efforts to see whether the Dispatch overloads from the middlewares can be used in the useDispatch definition, but I am not sure if that works.

[bfricka]

@Yakimych For me there's a simpler way to type mapDispatchToProps:

import {ResolveThunks, connect} from 'react-redux';
import React, {FC} from 'react';
// Normal action creator, taking string payload, and returning action.
// This is a typed action (ActionWithPayload<T>)
const fooAction = actionCreator<string>('FOO TYPE');
// Thunk that returns a Promise<string> (MyThunk is just an alias that
// fills in ThunkAction args with defaults for State and Action so it's less cumbersome)
const barThunkAction = (): MyThunk<Promise<string>> => = async (dispatch, getState) => ({});

interface ComponentRawDispatch {
  fooAction: typeof fooAction
  barAction: typeof barThunkAction
}

// These are your resolved props
type ComponentDispatchProps = ResolveThunks<ComponentRawDispatch>;

const Comp: FC<ComponentDispatchProps> = (p) => {
  p.fooAction // has type: (payload: string) => {type: string, payload: string};
  p.barAction // has type: Promise<string>
};

const mapDispatchToProps = {
  fooAction,
  barAction: barThunkAction,
};

export const ConnectedComp = connect(null, mapDispatchToProps)(Comp);

Typing it this way unpacks the actions correctly. The call to connect takes on the signature:

Connect.(
  mapStateToProps: null | undefined,
  mapDispatchToProps: MapDispatchToPropsParam<TDispatchProps, TOwnProps>
): InferableComponentEnhancerWithProps<ResolveThunks<TDispatchProps>, TOwnProps>;

The downside here is that you can miss required dispatch props b/c you're relying on inferring mapDispatchToProps (rather than typing it explicitly), however, in practice I haven't found this problematic b/c you'll know as soon as you try to use ConnectedComp that props are missing. However, there's probably a decent way to type this as well, I just haven't needed to.

Using hooks instead of connect should dramatically simplify this, as a well-typed useDispatch should properly unpack thunks the same way connect currently does with mapDispatchToProps. Using my example (without the ResolveThunks shenanigans), that means a proper dispatch looks like:

const Comp: FC<CompProps> = () => {
  const dispatch = useDispatch();

  fooAction('Foo'); // {type: 'FOO TYPE', payload: 'Foo'};
  dispatch(fooAction('Foo')); // {type: 'FOO TYPE', payload: 'Foo'};

  barAction(); // (dispatch, getState) => Promise<string>;
  dispatch(barAction()); // Promise<string>;
};

[bfricka]

I just plugged in the defs provided by @MrWolfZ and useDispatch is not going to unpack the way I described in my last post. In order to do so it'll probably have to use HandleThunkActionCreator. If you want me to help w/ this LMK (I'm not a TS ninja though).

[perrin4869]

@MrWolfZ

@le0nik the current design is a trade-off. It makes what we consider to be a fairly common pattern of selecting pieces of the state into an object easy and performant. Sure, if you select an array or object with millions of members, you might get into issues, but a) is this really a common use case, and b) if you have this kind of scenario, you can easily work around this like you showed in [2].

Isn't it best practice for selectors to be granular? You say that you select pieces of the state into an object, and yet I believe that in the world of hooks, thou should encourage multiple calls to useSelector instead.

If you want to select an object, I think it should be the developer's responsibility to memoize it, as opposed to having to work around it.

[threehams]

@bfricka I submitted two PRs which should take care of this, though I'm unsure of side effects this could have:
https://github.com/DefinitelyTyped/DefinitelyTyped/pull/34913#issuecomment-489268846

[josepot]

I couldn't agree more with @perrin4869 and @le0nik .

Hooks are composable, and given the fact that useSelector can return anything, if you want to build an object from the results of different selectors, you could do that by composing different useSelector hooks:

function MyComponent () {
  const foo = useSelector(fooSelector);
  const bar = useSelector(barSelector);
  const myObject = useMemo(() => { foo, bar }, [foo, bar]);
  // ...
}

Or you could still compose those granular selectors outside the render with some utility like Reselect:

const myObjectSelector = createStructuredSelector({
  foo: fooSelector,
  bar: barSelector,
});

function MyComponent () {
  const myObject = useSelector(myObjectSelector);
  // ...
}

The:

fairly common pattern of selecting pieces of the state into an object easy and performant

should now be accomplished by composing granular hooks. I think that the useSelector hook should be designed with that goal in mind. That's why I would also rather keep useSelector as minimalistic as possible.

If this was actually an issue, then we could always add this behavior later. However, once this behavior is added, then it becomes a lot more difficult to remove.

Also, the main reason why this has been a common pattern until now is that with connect we had to use a selector that returned an object... but that is no longer the case.

[joshjg]

@MrWolfZ

the current design is a trade-off. It makes what we consider to be a fairly common pattern of selecting pieces of the state into an object easy and performant.

This is a common pattern for the connect HOC because it is the _only_ way. You have to provide props to the wrapped component. But with hooks, we aren't dealing with props. As others have said we can encourage users to call useSelector multiple times in the same component with more granular selectors. I consider this analogous to useState, in which React decided to forego a shallow equality comparison for this same reason. So the concept should already be familiar to hooks users.

[threehams]

in which React decided to forego a shallow equality comparison for this same reason

React also added useReducer specifically because of readability issues from calling useState repeatedly within a component. Redux and local state are also nowhere near comparable in the scale of data they're meant to work with.

In use, I'm failing to see how this:

  const { tool, undoSteps, redoSteps, zLevel } = useSelector(
    (state: State) => ({
      tool: selectTool(state),
      undoSteps: state.tiles.past.length,
      redoSteps: state.tiles.future.length,
      zLevel: state.tiles.zLevel,
    }),
  );

is made better by being forced to do this:

  const tool = useSelector((state: State) => selectTool(state));
  const undoSteps = useSelector((state: State) => state.tiles.past.length);
  const redoSteps = useSelector((state: State) => state.tiles.future.length);
  const zLevel = useSelector((state: State) => state.tiles.zLevel);

[josepot]

@threehams

The thing is that if you actually needed a hook that performs a shallow compare, you could easily build it using a useSelector hook that works with strict equality, for instance:

const getShallowMemoizedSelector = selector => {
  let latestResult;
  return state => {
    const result = selector(state);
    if (shallowEqual(result, latestResult) {
      return latestResult;
    }
    return (latestlResult = result);
  }
}

const useObjectSelector = selector => {
  const memoizedSelector = useMemo(
    () => getShallowMemoizedSelector(selector),
    [selector]
  );
  return useSelector(memoizedSelector);
}

However, there is no way to do it the other way around. That's because a "strictly equals" useSelector is a better primitive.

Edit:

Also, this is not that bad:

  const tool = useSelector(selectTool);
  const tiles = useSelector((state: State) => state.tiles);

  const {zLevel, future: {length: redoSteps}, past: {length: undoSteps}} = tiles;

:see_no_evil: (ok, maybe it is :sweat_smile:)

[MrWolfZ]

I see where the people that don't want shallow comparison are coming from, but I am still in favor of keeping it. Here's why:

• it's how connect currently works, so it is going to make migrating to hooks a bit onerous, since you need to split your mapStateToProps into multiple useSelector calls
• it easily allows both patterns of using a single useSelector with an object and multiple useSelector calls without needing to create any custom hooks or using memoizing selectors
• a lot of the arguments here are in the style of "this is how you are supposed to use hooks, so let's encourage users to do it correctly". On the surface this seems like a good idea, but the issues I see are twofold: a) without the shallow comparison you are not just encouraging the multiple hooks style, you are actively punishing the single object useSelector style (by always forcing the component to re-render on each change which is much worse for performance than to compare objects with even a few thousand keys), and b) since when is it the task of this library to tell users how to write their code? Libraries should be about enabling the developer, giving them a choice, while preventing them from shooting themselves in the foot. Sure, there are situations where the current design is sub-optimal, but by and large it will just work without the developer needing to change the way the like to write code

• if you actually encounter a situation where this is significantly detrimental to performance, the workaround is absolutely trivial:

  // transform this
  const itemIds = useSelector(state => Object.keys(state.items))
  
  // to this
  const { itemIds } = useSelector(state => ({ itemIds: Object.keys(state.items) }))
  
  // or create a custom hook for this in one line
  const useObjectIsSelector = selector => useSelector(state => ({ result: selector(state) })).result
  

  Compare this to having to use a memoizing selector as shown by @josepot.

• I still doubt this will cause issues in practice. Have any of the apps you are working on a situation where this noticeably impacts performance? If so, could you please adjust the react-redux benchmarks to showcase this situation and how much worse it performs?

All this said, I am not the maintainer of this repository, so the decision is not up to me.

[josepot]

@MrWolfZ

it's how connect currently works, so it is going to make migrating to hooks a bit onerous, since you need to split your mapStateToProps into multiple useSelector calls

The argument of "it's how connect currently works" has already been refuted many times in the past. If that argument had any validity we would still have useActions. Which BTW I would like to have it back, but for different reasons. I do think that it's important that when we design the hooks API, we try to do so thinking how we would like to do things with hooks, without having to worry about how things were being done before we had hooks.

Compare this to having to use a memoizing selector as shown by @josepot.

Leaving aside the unnecessary stress that the useObjectIsSelector hook that you are proposing would put on the GC by generating a tone of unnecessary objects every time that a dispatch takes place. Please compare the names of these 2 different hooks: useObjectSelector vs useObjectIsSelector. The first one describes what the user is trying to select (not how it's implemented), the second one just describes an implementational detail. I think that with those examples you are inadvertently implying that the current implementation of useSelector with shallow-compare is kinda leaking an implementational detail.

I still doubt this will cause issues in practice. Have any of the apps you are working on a situation where this noticeably impacts performance?

Good news is that we have react-redux-benchmarks. So, there is no need to speculate on this. We can just test it :slightly_smiling_face:. For a different reason I've been refactoring the sources of the benchmarks so that they use the new hooks API. So, once I'm done, I won't mind running some tests for comparing the 2 different implementations.

[MrWolfZ]

@josepot I had already updated the benchmarks repo to hooks some time ago, so no need to do it again

[Yakimych]

@MrWolfZ

they should work just fine without any adjustments to the signature of useDispatch (granted, that's because it defaults to just any, but since any middleware can potentially accept any value, that's just how it is)

Both yes and no. It is possible to make sure we only call dispatch on "normal" actions and thunks:

In order to do this, we would have to annotate dispatch with ThunkDispatch<AppState, void, Action1 in mapDispatchToProps. And we can do this because we know that we're using redux-thunk middleware, so we just tell TS via a type annotation. Is there any way to achieve the same thing now when we can't really annotate useDispatch? I suppose the only way to get this to work with TS would be to create a new function for this: useThunkDispatch?

As far as I can tell, the use-cases for simple actions work as before. E.g. those would give the same errors with useDispatch?

@bfricka Wow, looks advanced 😃 I've never seen InferableComponentEnhancerWithProps - do you have tips on good resources where I can read up on those? I've created a repo with examples of how I usually type those things: https://github.com/Yakimych/redux-typescript-examples/blob/master/src/redux/chunk1.ts#L35

I threw it into a codesandbox, but not sure how to get TS autocomplete/errors in there: https://codesandbox.io/s/3v34zz97vm

[Yakimych]

@markerikson @timdorr I see this as feedback for the hooks API, since there are differences from how to type this compared to connect, but let me know if you don't want to overload this thread with TypeScript and if I should open a different issue or something 🙂

[threehams]

I think your specific issue's more related to our work in progress DefinitelyTyped issues. Once they're stable though, I'd be willing to write a short TypeScript section for the docs, or maybe just link to another page we can maintain and explicitly say "don't ping Mark for these things."

[josepot]

Hi all,

A couple of days ago @lukaszfiszer made the following comment in which it was suggested that it would be better to remove the dependencies from the useSelector API and let users memoize selectors themselves if they have to.

Three other people made comments expressing their agreement with that comment: @MrWolfZ , @Satyam and myself. Also, a few others reacted with a thumbs-up emoji to the comment, and so far no one has expressed any kind of disagreement or worries with that suggestion.

However, that is an important change in the API of useSelector and it's important to know whether there is a general agreement on that topic. So, if you have any concerns with that suggestion, please express them :slightly_smiling_face:.

Also, I think that it would be nice to know what the main maintainers of the library think about that suggestion :pray: cc @markerikson @timdorr

[dkadrios]

How would such a scheme work with something like reselect style selectors?

via mobile

On Sat, May 4, 2019, 1:26 PM Josep M Sobrepere [e-mail]
wrote:

Hi all,

A couple of days ago @lukaszfiszer https://github.com/lukaszfiszer made
the following comment
https://github.com/reduxjs/react-redux/issues/1252#issuecomment-488591342
in which it was suggested that it would be better to remove the
dependencies from the useSelector API and let users care about memoizing
the selectors themselves if they have to.

Three other people made comments expressing their agreement with that
comment: @MrWolfZ https://github.com/MrWolfZ , @Satyam
https://github.com/Satyam and myself. Also, a few others reacted with a
thumbs-up emoji to the comment, and so far no one has expressed any kind of
disagreement or worries with that suggestion.

However, that is an important change in the API of useSelector and it's
important to know whether there is a general agreement on that topic. So,
please, if you have any concerns with that suggestion, please express them
🙂.

Also, I think that it would be nice to know what the main maintainers of
the library think about that suggestion 🙏 cc @markerikson
https://github.com/markerikson @timdorr https://github.com/timdorr

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/reduxjs/react-redux/issues/1252#issuecomment-489362858,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAJKXKY374G4LYSN6MO4II3PTXWPFANCNFSM4HHQNECA
.

[josepot]

@dkadrios

How would such a scheme work with something like reselect style selectors?

Reselect style selectors are already memoized outside of React, so there would be nothing to be done :slightly_smiling_face: Just put the selector inside useSelector and you are done.

Perhaps your question is about how selectors that are built with Reselect and that accept props will work after this change... But this change wouldn't affect those cases that much either. For those kinds of selectors, without this change you would have to do something (more or less) like this:

const selectedValue = useSelector(
  state => selectorThatAcceptsProps(state, props),
  [props.idOrThePropsThatYouCareAbout]
);

and if that proposal gets accepted you would have to do something like this:

const selector = useCallback(
  state => selectorThatAcceptsProps(state, props),
  [props.idOrThePropsThatYouCareAbout]
);
const selectedValue = useSelector(selector);

Although, if you have a lot of those cases, you could also consider making a custom hook named useInstanceSelector or something like that to avoid boilerplate.

Also, in this other comment I explain how I would do it if I were using Reselect with factory-selectors.

Although, TBH it would have to be Reselect's responsibility to update their examples for how to leverage their API with hooks, and perhaps even provide with the custom hook for working with "shared selectors".

In any event, IMO this change wouldn't make things more difficult for Reselect users. Believe me, I've been using Redux for some years now and I have always used it with Reselect :sweat_smile:.

[markerikson]

Quick personal status update:

I am _swamped_ with day job work right now, so I don't have any real time or mental capacity to think about this at the moment. I'm hoping things will calm down in the next couple weeks.

I know there's a bunch of outstanding PRs and discussion, but y'all will have to be patient :)

[timdorr]

I'm similarly behind on reading through this ASOIAF-scale amount of comments and code. Unfortunately, I don't have the excuse of my job eating up my time; I'm just lazy. I'll try to binge-read everything soon!

[timdorr]

So, I'm pretty split on the whole memoization issue. It would be really nice if there was a useProps, because then we could mirror the mSTP behavior of checking for a second arg on the function. Oh well...

I think I'm in favor of removing it. The whole point of adding Hooks isn't to make users recreate connect inside of their components; it's to allow a little bit of Redux usage anywhere without having to break out a heavyweight tool. Very often you will be using one Hook at a time. If you're using useSelector and useDispatch, you're basically a step away from needing connect anyways, so just use that.

I think we need to be pretty clear: This is not a replacement for connect. Let's keep that in mind as we discuss this stuff too.

[mdcone]

Writing tests in enzyme has been easy for components wrapped in a connect because we can pass in the store as a prop and then do a shallow render.

When using the hooks API, however, we are forced to wrap the code in a Provider in order to supply the store to the context. I won't get into the details here about how we are using enzyme, but this makes testing much harder to do.

Is there a way that the store can be set for the hooks API in a testing scenario? Is that something that should be considered at all?

[timdorr]

Nope, not being considered yet. You should stick with connect for those kinds of things.

One possible thing would be to let you pass in a store to the hooks via props:

function MyComp({ store }) {
  const val = useSelector(mySelector, store)

And we just check for either arguments.length or store === undefined.

[MichaelDeBoey]

@mdcone If you need to change your tests because you're changing to hooks, you're probably testing implementation details.

A way to be sure you're not testing implementation details is to try and use the great react-testing-library @kentcdodds provided to the community. 🙂

[retyui]

How about use declarative style for useSelector:

const getUserName = (store, id) => {/* ... */};

const name = useSelector(getUserName, [props.userId]);

[mdcone]

@timdorr, that dependency injection approach would work. Feels weird supplying the store as a prop, but maybe that's the tradeoff that would need to be made.

@MichaelDeBoey, thanks for the reference, I'll check it out.

@josepot The root of the matter is that when we render out a component using enzyme, and it is wrapped in a Provider, and then we dive into that component to verify some of its children, react-redux loses the context the Provider had set and throws an exception.

This could just be an issue with how enzyme works or how we are using it, but something I noticed yesterday when using these new hooks and writing a test for the component using them.

Thanks for the feedback and this library!

[josepot]

One possible thing would be to let you pass in a store to the hooks via props

I really dislike this idea.

Adding this to the API just to provide a means for writing those kinds of tests seems wrong. Also, the only way to take advantage of that would be to change the API of the component so that it can receive the store through props just for testing purposes...

This problem can be easily fixed from the tests, without the need of having to write all your components in a super-weird way just for testing purposes. I mean, if you have to change the API of all your components just so that you can test them... Perhaps you should reconsider the way that you are testing them.

Instead of changing the API of all your components, why not use a HOC like this in your tests?

const withStore = store => Component => props => (
  <Provider store={store}>
    <Component {...props} />
  </Provider>
);

Also, as @timdorr already said before:

This is not a replacement for connect. Let's keep that in mind as we discuss this stuff too.

[josepot]

Hi @mdcone

@josepot The root of the matter is that when we render out a component using enzyme, and it is wrapped in a Provider, and then we dive into that component to verify some of its children, react-redux loses the context the Provider had set and throws an exception.

This could just be an issue with how enzyme works or how we are using it, but something I noticed yesterday when using these new hooks and writing a test for the component using them.

Is this code public? Or could you please show the relevant parts of this somewhere? I would like to have a look at it because I really think that this is an XY problem. Also, I agree with @MichaelDeBoey check this out.

[mdcone]

@josepot It isn't public, but here's an example:

test('The component outputs a div element', () => {
        const store = generateStoreWithData({
            message: 'Howdy'
        });

        function MessageComponent() {
            const message = useSelector(state => state.message);

            return <div>{message}</div>;
        }

        const renderedWithProvider = shallow(
            <Provider store={store}>
                <MessageComponent />
            </Provider>
        );

        expect(renderedWithProvider.find('MessageComponent').length).toBe(1);

        const messageComponent = renderedWithProvider
            .find('MessageComponent')
            .dive();

        expect(messageComponent.find('div').length).toBe(1);
    });

The error produced is "Invariant Violation: could not find react-redux context value; please ensure the component is wrapped in a " at the point where the dive is called.

[josepot]

@mdcone

I'm not an enzyme expert, but I think that for those tests you should use mount instead of shallow.

According to the enzyme docs:

• shallow:

Shallow rendering is useful to constrain yourself to testing a component as a unit, and to ensure that your tests aren't indirectly asserting on behavior of child components.

• mount:

A method that re-mounts the component, if it is not currently mounted. This can be used to simulate a component going through an unmount/mount lifecycle.

No equivalent for ShallowWrappers.

TBH I'm surprised that shallow works with connected components :man_shrugging:

Although, please notice that even the React docs recommend using react-testing-library. IMO it's a much better testing approach than using enzyme.\

EDIT

A few things that I've learned from this:

1) It's dangerous to give advice when you don't fully know other ppl situation. I didn't know that you were using React Native. It is true that this library exists, but I suspect that most RN developers are still using enzyme... If I had known that, then I would have thought it twice before "pushing" you out of enzyme. My bad for being so judgemental.

2) It turns out that I got lucky with my "mount" suggestion because those tests do work using mount.

3) Despite the fact that the tests are working, I can see this message in the console:

Warning: unmountComponentAtNode(): The node you're attempting to unmount was rendered by another copy of React.

This makes me feel suspicious that enzyme must be using its own version of React, and as we all know hooks don't work properly when there are duplicate React versions. So, I checked if enzyme's latest version fully supports hooks and it turns out that it doesn't yet.

4) It's quite likely that once enzyme fully supports hooks you won't have this issue. So, if that is the case, then this is, in fact, an XY problem. In any event, this is a tooling issue, you can setup enzyme to work with mount. It does look painful, but it can be done :slightly_smiling_face:.

[mdcone]

@josepot Thanks for the advice. We're running in React Native, so I'm not sure that mount works without hacking a solution together because it wants a DOM to attach to, but I do appreciate you taking the time to help out.

Sounds like using a tool other than enzyme will be worth evaluating going forward!

[AlicanC]

I'm having a problem with useSelector().

Example:
https://codesandbox.io/s/925no65j4w

Explainer:
I have a map (new Map()) in my state and I have an action that sets values in it by cloning the map, setting the value and putting the clone in the next state.

When I read the map using myMap = useSelector(state => state.myMap) my component doesn't get updated so I have to write { myMap } = useSelector(state => state) instead.

[markerikson]

@AlicanC : I think I know why that's happening, and it makes sense to me that you're seeing that.

You're creating a new Map instance each time. However, as a class instance, all of its methods are actually on the prototype. Also, all the data is stored internal to the map. So, two different Map instances are actually shallow-equal to each other, and useSelector() is not triggering an update. Putting the Map instance into an object as a field causes the returned object to not be shallow equal to the last result.

As a side note, we would generally recommend _not_ putting Maps or other non-serializable values into your Redux store. The code may run, but you're more likely to encounter problems like this.

[josepot]

@markerikson @AlicanC

As a side note, we would generally recommend not putting Maps or other non-serializable values into your Redux store. The code may run, but you're more likely to encounter problems like this.

IMO a Map is pretty easy to serialize / deserialize. Just because it's not a plain javascript object does not mean that it's not serializable. Some people even put ImmutableJs instances in their redux-store. Although, I do agree that for that case using an Object makes more sense.

To me, what's clearly problematic is having useSelector perform a shallow-equal, instead of performing an Object.is or a strictly equal. :man_shrugging:

IMO this issue highlights what I already said before: which is that performing a shallow-equal comparison on useSelector creates a leaky abstraction, because it forces developers to understand a pretty arbitrary implementation detail.

[artem-malko]

Great feature!
I have just one question: how can I apply factory mapStateToProps flow to useSelector? I mean https://react-redux.js.org/api/connect#returns (text from note).

[markerikson]

@artem-malko : you would want to generate your own selectors that capture props and memoize that with useMemo() or useCallback():

function MyComponent(props) => {
    const selector = useCallback(
        (state) => state.todos[props.id],
        [props.id]
    );

    const todo = useSelector(selector);
}

On which note, it looks like we don't have an example like that in the hooks alpha docs page. Either we had one and it got removed, or we never put one in. We should show that.

[timdorr]

That should show up in #1267 now that I've merged into the deps removal stuff.

[MrWolfZ]

I am currently on a business trip and don’t have access to my normal work environment. I'll update the docs as soon as I can (probably next weekend) with the removal of useActions and deps.

[Lenic]

I think this is the right way:

// mycode.js
function MyComponent(props) => {
  const todo = useSelector(state => state.todos[props.id], state => [props.id, state.todos]);
  // other code
}

And the useSelector cache selector parameter like this:

// useSelector.js
export function useSelector(selector, depsFunc) {
  const { store, subscription: contextSub } = useReduxContext();

  // compute dependency by current state
  const deps = depsFunc(store.getState());

  const latestDeps = useRef(null);
  const latestSelectedState = useRef(null);

  if(latestSelectedState.current !== null && latestSelectedState.current !== null && equalDeps(deps, latestDeps.current)) {
    return latestSelectedState.current;
  }

  latestDeps.current = deps;
  const currentSelector = useCallback(selector, deps);
  // other code

[josepot]

@Lenic

1272 was recently merged, which removes the deps argument of useSelector. In that PR there is the following comment from @gaearon that I think summarizes the rationale for this change very well:

FWIW our conclusion internally has been that custom Hooks should avoid their own dependency argument, and people should just memoize if it's necessary. You have to learn that pattern anyway.

Also, I don't understand what you are suggesting. I think that the only valid reason for changing the selector provided to useSelector is when the selector depends on props. See this comment from @timdorr and the answer that follows. The depsFunc that you are proposing is just another selector... Which means that logic could be encapsulated in just one function that composes those 2 selectors. IMO that makes depsFunc a superfluous argument.

[Lenic]

@josepot

I understand what @gaearon means, the useSelector api is the same as the connect function, and the extra caching is provided by the third-party library.

My idea is to provide an api similar to React Hooks, with dependencies to decide whether to recalculate the results.

// useCacheSelector.js
export default function useCacheSelector(selector, depFunc) {
  const store = useStore();
  const deps = depFunc(store.getState());

  return useMemo(()=> useSelector(selector), deps);
}

This way, I can get the cached value if the dependency has not changed. I am doing the job like a reselect library.

// mycode.js
function MyComponent(props) {
  const todo = useCacheSelector(state => state.todos[props.id], state => [props.id, state.todos]);
  // other code
}

[istarkov]

It seems for me that useLayoutEffect here https://github.com/reduxjs/react-redux/blob/a787aeeb5ab064e66ff168eedbb962d3397b378d/src/hooks/useSelector.js#L81
is not needed and can be changed to useEffect.

As even you lost event in between render and useEffect then checkForUpdates will do all the dirty work https://github.com/reduxjs/react-redux/blob/a787aeeb5ab064e66ff168eedbb962d3397b378d/src/hooks/useSelector.js#L105
In case of useEffect you will need to check didCancel in event handler see this

As for now the only test which will fail is https://github.com/reduxjs/react-redux/blob/a787aeeb5ab064e66ff168eedbb962d3397b378d/test/hooks/useSelector.spec.js#L50-L76 and it's check some internal implementation details without any reason I know.

[markerikson]

@istarkov : I believe that's there because we had to modify connect() to use layout effects for its subscriptions to solve some timing bugs, so we're doing the same thing here for consistency.

[MrWolfZ]

@istarkov your observations are indeed correct, as is @markerikson's explanation as to the why.

While there is no technical need to use a layout effect, I thought it makes sense for consistency. Since there isn't really an observable difference, the test checks that implementation detail. However, we could easily change it if necessary. Do you know of a good reason we should prefer useEffect over useLayoutEffect?

Also, maybe we should add a comment to that effect that explains this so that the question doesn't come up again in the future.

[istarkov]

Thank you for the answers!

Do you know of a good reason we should prefer useEffect over useLayoutEffect`?

In case of current code and React version - no. There are no heavy computations inside, having sync nature seems like this https://github.com/reduxjs/react-redux/blob/a787aeeb5ab064e66ff168eedbb962d3397b378d/src/hooks/useSelector.js#L105 can be deleted also useLayoutEffect highly simplifies code and thinking about, also its somehow "system" level library so all is fine.

For me the most issue is incorrect comment https://github.com/reduxjs/react-redux/blob/a787aeeb5ab064e66ff168eedbb962d3397b378d/src/hooks/useSelector.js#L9-L14 as here useLayoutEffect is used just to hold callbacks and data references as an optimisation hack.
As for now useLayoutEffect looks like the best way to avoid additional use{XXX}Effect dependencies for optimisation purposes.

As like many others before writing my code I'm using github to find similar code for ideas, good practices, possible edge cases etc - so this comment made me think that there is something exists I don't understand ;-) and the best way to learn is to ask here, thank you again!

[timdorr]

From @iamawebgook here https://github.com/reduxjs/react-redux/issues/1286#issuecomment-493325008

With connect you can pass manual equality checking function, instead of shallowEqual, but there is no way to do it with useSelector, there is no way to fix these kind of cases and turns out I need to rewrite lots of my code, because I have different data structures used on my components. Also transforming to other data types may be overhead. I suggest to add some way to support manual comparison of values for useSelector.

I do think this is something we should add. Maybe as an options arg?

[MrWolfZ]

@timdorr yeah, in regards to the comparison I was thinking as well about making this an option. At the same time, I want the API to be as simple as possible, and if a custom comparison is required, one can always just use connect.

[markerikson]

After a bunch of discussion on equality behavior over in #1288 , I've just published v7.1.0-alpha.5, which:

• makes reference equality the default
• adds an optional comparison function as the second arg to useSelector()
• exports our shallowEqual function for use with useSelector() if desired

[mfahl]

Using typescript and "react-redux": "^7.1.0-alpha.5" "@types/react-redux": "^7.0.9" I get a missing type error:
Module '"../../../node_modules/@types/react-redux"' has no exported member 'useSelector'. TS2305

Does @types/react-redux already include the types for hooks such as useSelector?

[luke-john]

@mfahl I was also looking at this too today, and just saw your comment added as i went to close this tab.

They don't yet, there's an active PR at this link, but in the meantime there's this comment on that pr which you can follow to get some typescript support in the meantime.

[josepot]

@mfahl and @luke-john

I'm not a TS expert, but I think that the correct typings after the release of v7.1.0-alpha.5 should be something like this:

import { Action, Dispatch, AnyAction } from 'redux'

declare module 'react-redux'
{
    export function useDispatch<A extends Action = AnyAction>(): Dispatch<A>
    export function useSelector<TState, TSelected>(
      selector: (state: TState) => TSelected,
      equalityFn?: (a: TSelected, b: TSelected) => boolean
    ): TSelected
    // ... other additions from the pull request
}

[TeoTN]

Can we have useAction(actionCreator) hook? (I don't mean the useActions that was removed) It's a bit mundane to type everywhere:

const dispatch = useDispatch();
const onWhatever = (...args: XYZType) => dispatch(actionCreator(...args));

[jamsch]

import { Action, Dispatch, AnyAction } from 'redux'

declare module 'react-redux'
{
    export function useDispatch<A extends Action = AnyAction>(): Dispatch<A>
    export function useSelector<TState, TSelected>(
      selector: (state: TState) => TSelected,
      equalityFn?: (a: TSelected, b: TSelected) => boolean
    ): TSelected
    // ... other additions from the pull request
}

Shouldn't equalityFn compare the root state and not the return value?

[markerikson]

@jamsch : nope. it's explicitly to compare the previous and current return value, to determine if it should force the component to re-render because the data changed.

[markerikson]

@TeoTN : odds are we won't ship useActions(), but there's a copy-pastable version in the docs page:

https://react-redux.js.org/next/api/hooks#recipe-useactions

For the binding part, you could also just import bindActionCreators() from Redux.

[emily-curry]

@TeoTN If it's the types tripping you up, it is relatively simple to create your own generically typed useAction hook:

export const useAction = <A extends any[], R extends AnyAction>(
  actionCreator: (...args: A) => R
): ((...args: A) => R) => {
  const dispatch = useDispatch();
  return useMemo(() => (...args: A) => dispatch(actionCreator(...args)), [
    dispatch,
    actionCreator
  ]);
};

The resulting function will have the same signature as the actionCreator you pass into it.

[TeoTN]

@markerikson I'm aware that the hook using action creator map was removed, I mean a hook for a single action creator
@emily-curry Thanks :) However, I'd prefer not to have it at all than to include a custom hook in every project. Maybe I'm wrong, but I feel that a dispatch without an action (creator) is not very useful, and what people care about is not the dispatcher function, but rather a function that would call a dispatcher with some action, and with that in mind, it'd be nice to have it in the library. Maybe the useActions wasn't ideal but useAction seems to be nearly equivalent to useDispatch yet more concise in its usage. If that's not an option, I don't want to grow utils files to substitute "missing" libraries features. ;)

[AlicanC]

@TeoTN they are not useful because they are generic. I actually wrap useSelector and useDispatch for every store in every project so they work on the actual state and dispatch of the stores:

Bad:

import { useSelector } from 'react-redux';

export default function MyComponent() {
  const state = useSelector((state) => state);

  // `state` is not useful because it's not typed properly
}

Good:

// File: store/useSelector.js
import { useSelector } from 'react-redux';

import { type State } from './state.js';

// Custom type that actually works on the state of my store
type UseSelector = <TMappedState>(mapStateToProps: (state: State) => TMappedState) => TMappedState;

// Cast stock useSelector to the custom type and export
export default (useSelector: UseSelector);

// File: components/MyComponent.jsx
import useSelector from '../store/useSelector';

export default function MyComponent() {
  const state = useSelector((state) => state);

  // `state` is typed perfectly
}

My examples are in Flow, but same should be doable in TS too. If you do the same for useDispatch, then you will be also getting auto-completion from your IDE when calling dispatch() which will eliminate the need for having actionCreators.

[markerikson]

@TeoTN : just to be clear - the useActions() hook we had / removed / show in the docs can handle three kinds of arguments:

• a single action creator function
• an array of action creators
• an object of action creators

[linvain]

Because of

export function useSelector(selector, equalityFn) {
  if (equalityFn === void 0) {
    equalityFn = refEquality;
  }

My IDE throws a warning "Expected 2 arguments" when i do

const products = useSelector(store => store.products)

[MrWolfZ]

@linvain Hmm, in the source code that function is using a default value which makes the parameter optional. Your code looks like a downleveled version of this, i.e. the ES5 version. What IDE are you using?Also, maybe it is the JSDoc comments tripping up your IDE? Are the comments present at the definition of the function for you? If so, what happens if you wrap the equalityFn param in angle brackets, i.e. like this?

* @param {Function} [equalityFn] the function that will be used to determine equality

[linvain]

@MrWolfZ

node_modules/react-redux/lib/hooks/useSelector

/**
 * A hook to access the redux store's state. This hook takes a selector function
 * as an argument. The selector is called with the store state.
 *
 * This hook takes a dependencies array as an optional second argument,
 * which when passed ensures referential stability of the selector (this is primarily
 * useful if you provide a selector that memoizes values).
 *
 * @param {Function} selector the selector function
 * @param {Function} equalityFn the function that will be used to determine equality
 *
 * @returns {any} the selected state
 *
 * @example
 *
 * import React from 'react'
 * import { useSelector } from 'react-redux'
 * import { RootState } from './store'
 *
 * export const CounterComponent = () => {
 *   const counter = useSelector(state => state.counter)
 *   return <div>{counter}</div>
 * }
 */


function useSelector(selector, equalityFn) {
  if (equalityFn === void 0) {
    equalityFn = refEquality;
  }

node_modules/react-redux/es/hooks/useSelector

/**
 * A hook to access the redux store's state. This hook takes a selector function
 * as an argument. The selector is called with the store state.
 *
 * This hook takes a dependencies array as an optional second argument,
 * which when passed ensures referential stability of the selector (this is primarily
 * useful if you provide a selector that memoizes values).
 *
 * @param {Function} selector the selector function
 * @param {Function} equalityFn the function that will be used to determine equality
 *
 * @returns {any} the selected state
 *
 * @example
 *
 * import React from 'react'
 * import { useSelector } from 'react-redux'
 * import { RootState } from './store'
 *
 * export const CounterComponent = () => {
 *   const counter = useSelector(state => state.counter)
 *   return <div>{counter}</div>
 * }
 */


export function useSelector(selector, equalityFn) {
  if (equalityFn === void 0) {
    equalityFn = refEquality;
  }

As you see in both files there is no default value for the parameter and the parameter isn't stated as optional in JSDoc

My IDE is IntelliJ IDEA. My package manager is Yarn

The warning disappears whether I make the parameter optional in JSDoc or add the default value for it in the code (my IDE uses the file in the es folder, so I edit that)

[MrWolfZ]

@linvain Alright, I've created a PR to mark the parameter as optional in the JSDoc comment. This should fix your issue with the next alpha. Temporarily you can just use the manually edited file in the es folder.

EDIT: someone else had already created a PR, so I closed mine.

[markerikson]

So, how are folks feeling about the hooks APIs overall at this point?

Any further major concerns? How's the useSelector equality change working out? What about working with selectors that rely on props?

I don't have a specific timeline for moving this from alpha to beta, but I'd like to start hearing a sizeable number of "looks good to me" responses as a signal that we're maybe ready to stabilize the APIs and move forward.

[h0jeZvgoxFepBQ2C]

Could you maybe add more info about redux hooks advantages to the docs? I'm a redux newbie (but worked already on some small apps with redux successfully) and right now I'm not sure anymore what's the actual advantage of using redux, since with hooks and contexts react already has many features out of the box without redux.

It would be also great if you could point out some best practice on when to use redux hooks vs dispatching actions? Hope this is somehow relating to this issue, if not then please ignore my question. Thank you so much for your great work (and I love redux btw :) )! ❤️ 👏 👏 👏

[markerikson]

@h0jeZvgoxFepBQ2C : mmm... that seems like it's mostly a separate question of "why use Redux vs anything else", not "why use the React-Redux hooks APIs vs the connect API".

I'd encourage you to watch my Reactathon 2019 talk on "The State of Redux", and read my post Redux - Not Dead Yet! as well as Dave Ceddia's post Redux vs the React Context API.

[henrikhermansen]

looks good to me

I have a branch sitting ready where we've moved a generic data component from connect to hooks. It's gotten a lot leaner and a lot faster. :rocket:

[mustafahlvc]

looks good to me

I have a branch too, that all codes migrated to the hooks. I don't have any problems, looking forward to new release.

[markerikson]

@henrikhermansen : out of curiosity, any specific metrics on "it got a lot faster"?

[henrikhermansen]

@markerikson I just used the React profiler on one of our heavier pages and saw it did fewer and faster re-renders while fetching data and populating the redux store. I don't have the numbers anymore, but it was a big difference.
This is just because the hooks makes the implementation of our data component so much easier and better than what it was with connect. So I'm not saying hooks performs better than connect, I'm just saying it fits our desires alot better :)

[njacob1001]

When will the version that supports the useSelector and useDispatch hooks be?

[markerikson]

@njacob1001 : No specific timeline - see https://github.com/reduxjs/react-redux/issues/1252#issuecomment-496025983 .

[artem-malko]

@markerikson how many "looks good to me" responses do you want to have to start beta-testing?)

[ee0pdt]

Adding my 'Looks good to me' response here... I do have some selectors that use props but I'm probably going to leave them in the old non-hooks format for now and worry about building new stuff with hooks

[MatthieuCoelho]

I migrated some class component to functional component using react-redux hooks.
The performance are about the same, but it is definitively easier / cleaner to use hooks than connect.
So it looks good to me.

[jamsch]

Refactored around 30 components on a react native app that uses Redux with useSelector/useDispatch. Performance seems fine. About to deploy the changes to ~50k users.

[neosiae]

I built a simple app with React hooks and Redux v7 hooks alpha. I must say, great job! Code is much easier to reason about.

[lucianoassis]

I have also refactored around 20 components.

The code looks fantastic. We didn't notice any difference in terms of performance so far.

We had some issues related to jest + enzyme but it seems to be a problem in enzyme itself (https://github.com/airbnb/enzyme/issues/2107).

[markerikson]

@artem-malko : exactly 4792 responses :)

@MrWolfZ , @josepot : you two have done a lot of the direct contributions to this. Thoughts?

[timdorr]

I'm good with it as-is. Let's release a beta.

[MrWolfZ]

I agree, I think we're good. Let's get it out there.

[markerikson]

Awright, let's do it.

I'm busy the rest of the day. Tim, you wanna hit the button on it?

[josepot]

I agree, I think we're good. Let's get it out there.

💯

[timdorr]

Yeah, I need to get to my other laptop. I'll do it tonight.

[timdorr]

I went bananas and made it an RC: https://github.com/reduxjs/react-redux/releases/tag/v7.1.0-rc.1

Since we don't need generalized feedback anymore, I'm going to close this out. But please feel free to open up new issues for specific problems or concerns you have about the new Hooks. Just because it's RC'd doesn't mean it's finished :)

[dimaqq]

I built a simple app with React hooks and Redux v7 hooks alpha. I must say, great job! Code is much easier to reason about.

@neosiae share please :)

[neosiae]

@dimaqq https://github.com/neosiae/moviedb

Keep in mind that useActions() hook was removed in v7.1.0-alpha.4.

[MGaburak-eleks]

Please add a Context parameter to hooks API: useSelector, useDispatch, etc...

[josepot]

Please add a Context parameter to hooks API: useSelector, useDispatch, etc...

@MGaburak-eleks why? What's the use case?

[alexej-d]

Please add a Context parameter to hooks API: useSelector, useDispatch, etc...

@MGaburak-eleks why? What's the use case?

I think I know what is meant. I had a suiting use case. Imagine different nested stores with the same keys and child components listening and changing the same keys but on different nesting levels. Sometimes you do not want to have one single store but context dependent.

[josepot]

I think I know what is meant. I had a suiting use case. Imagine different nested stores with the same keys and child components listening and changing the same keys but on different nesting levels. Sometimes you do not want to have one single store but context dependent

Can't you do that by using different redux Providers?

Although, in all honesty, if you are actually using different stores for different "contexts", I wonder whether Redux is the right tool for what you are doing...

[markerikson]

@josepot : no. If you look way back in the previous hooks thread, I originally proposed adding a customContext parameter to the hooks to allow this customization, and the consensus was that wasn't worth including. So, our useStore() hook directly pulls in the singleton ReactReduxContext instance and reads the store from that, and all the other hooks rely on useStore(). If you do supply a custom context to <Provider>, the hooks currently have no way of accessing the store from that context instance.

It's a tradeoff - simplicity vs customization.

[MGaburak-eleks]

On the documentation page https://react-redux.js.org/next/api/connect#options-object connect support context parameter I think hooks also should support this feature.

[markerikson]

I don't think we're going to add that for the initial release.

_If_ there's sufficient demand for it, we may consider it down the road.

@MGaburak-eleks : what is your specific use case for needing a custom context parameter?

[leoyli]

@markerikson, @timdorr,

Thank you for these amazing work ❤️

Since RC.1 was out almost a week and look like there wasn't any blocker, do you guys have a targeting date for the final release? 😃

[lrdxbe]

I would really love to make the switch but right now I'm still struggling figuring out how my test will be made. I'm using TypeScript and react-testing-libary. Right now testing connected components is easy: I export the "raw" component and test it instead of the connected one.

type Todo = {
  id: number;
  text: string;
};

type AppState = {
  todos: {
    list: Todo[]
  }
}

type TodosProps = {
  addTodo: (todo: Todo) => void;
  todos: Todo[];
};

const TodosComponent: FC<TodosProps> = ({ todos, addTodo }) => {
  const list = todos.map(todo => <p key={todo.id}>{todo.text}</p>);
  const randomTodo: Todo = {
    id: Math.ceil(Math.random() * 1000),
    text: "A new one"
  };
  return (
    <>
      <p>
        <button type="button" onClick={() => addTodo(randomTodo)}>
          Add a todo
        </button>
      </p>
      <h3>List</h3>
      {list}
    </>
  );
};

const mapStateToProps = (state: AppState) => ({
  todos: state.todos.list,
});

const mapDispatchToProps = {
  addTodo: addTodoActionCreator,
};

const Todos = connect(mapStateToProps, mapDispatchToProps);

Here I could easily test TodosComponent and pass it jest.fn() for addTodo (dispatch props) and an array of my choice for its todos (state props). I don't need to create a store with a correctly typed state just to test this component.

But with hooks, I cannot exract redux from my component. Which means that I need to create a store with a correctly typed state (optionnaly hydrated with the values I want). This doesn't seem convenient and make my tests consume way more external dependencies (redux, action creators, selectors) which isn't arguably that bad (since react testing is almost always integration testing anyway).

I tried to found resources on the matter but couldn't, I guess it's normal since the use of hooks in Redux is not even officially release yet. But I wonder if you guys have any leads on this?

[markerikson]

@lrdxbe : yeah, as @timdorr just said over in #1001 , it would be great if we could get some docs on testing added.

That said, I personally don't have the experience (or time) to write those. I'd really appreciate it if someone else could come up with some good strategies and file a PR documenting those.

[lukaszfiszer]

@lrdxbe if you want you can still create a "connected" version of TodosComponent using hooks:

const Todos = () => {
  const todos = useSelector(state => state.todos.list);
  const dispatch = useDispatch();
  const addTodo = useCallback((todo) => dispatch(addTodoAction), [dispatch]);
  return <TodosComponent todos={todos} addTodo={addTodo} />
}

However I'd recommend against using that. Testing TodosComponent in isolation from Redux store does not give you any confidence that the component really works in the context of your application. You should test Redux integration on some level, and integration testing with RTL seems like a ideal place to do it.

[lrdxbe]

I don't really see the point in using a container component just to do that either. Like Kent C. Dodds said, this Container/Presenter pattern doesn't make a lot of sense in the age of hooks.

I could (and maybe will be forced to do that) test my connected components with all the store/reducers/action creators/selectors logic but that would be tedious and I've already got dedicated tests for those. Testing everything together is already been made with e2e tests. While I get that react testing is mostly integration testing (I'm embracing the react-testing-library philosophy), I still feel like some level of isolation would be needed for UI testing.

[SeanRoberts]

Hey that was my AMA question for Kent... I'm on TV!

For testing redux connected components I use my app's custom test render function that sets up all my necessary providers and takes options to hydrate each of them. I'm also using Typescript and also didn't want to have to deal with passing a fully hydrated store just to test some tiny slice of it in relative isolation so in my test render function I just typed it as any. Having a fully typed store is great when I'm running my full app because it saves me from making big mistakes but I don't mind using the any escape hatch in my integration tests because if I make a mistake my test is going to fail anyway. Once I hydrate the initial state that a component needs all the action creators/selectors/etc. just work.

[Joonpark13]

Sorry to chime in late, but I wanted to post a note for @mdcone or anyone else who might be coming to this thread for questions with usage with enzyme, as I just did: I've found that if you spy on useSelector and useDispatch, the provider issues go away. I understand that some may argue this isn't great practice, but it's obvious that there are at least a handful of us that are out there who are bound (at least for the time being) to large codebases at work that would be very difficult to migrate testing libraries, and for me this approach has worked fine (and I find myself needing to assert on spy calls anyway).

But I, too, am definitely interested in learning more about react-testing-library and its more integration focused approach! On my to-learn list 😄