dai-shi/reactive-react-redux

Discussion: Antipattern to recommend `useTrackedSelector`

JackCA opened this issue · 4 comments

I'm in the process of migrating to this library from react-redux (it's made a very large positive performance impact) -- but I noticed that the recipe for useTrackedSelector contains a useTrackedState() call inside of it.

If a developer who has components with muliple useSelector calls were migrating from react-redux and implemented this recipe, wouldn't they violate the recommendation to only use useTrackedState() once in each component?

Good point.
In general, I'd recommend using useSelector in reactive-react-redux, when migrating from react-redux. And, then convert some of them into useTrackedState and/or useTrackedSelector.

Having multiple useTrackedSelector (and thus useTrackedState) in a component is technically valid as long as you don't hit that caveat's case.
That is:

const foo = useTrackedSelector(state => state.foo);
const foo2 = useTrackedSelector(state => state.foo);
if (foo !== foo2) {
  return 'should not happen in react-redux, but could happen in reactive-react-redux';
}

It's not likely to write such code. I might be missing some other common cases that cause this inconsistent behavior though.

Do you have any suggestion to improve the description in README?

Here's another recipe.

// selector definitions
const selectCount = (state) => state.count;
const selectText = (state) => state.text;

// in react-redux
const count = useSelector(selectCount);
const text = useSelector(selectText);

// in reactive-react-redux
const state = useTrackedState();
const count = selectCount(state);
const text = selectText(state);

// in reactive-react-redux with a hack
// --- define a custom hook that returns a hook-like function
const useUseTrackedSelector = () => {
  const state = useTrackedState();
  return (selector) => selector(state);
};
// --- using this custom hook
const useSelector = useUseTrackedSelector();
const count = useSelector(selectCount);
const text = useSelector(selectText);

I think that is a lot better! 👍

Thanks for the thoughtful response. I don't think it's likely to hit that situation but this recipe should help mitigate that.

Cool. I will add a link in README to this issue for the time being.