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 (

) }`

[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 (

)`

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