Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Discussion: Antipattern to recommend useTrackedSelector #41

Closed
JackCA opened this issue Dec 21, 2019 · 4 comments
Closed

Discussion: Antipattern to recommend useTrackedSelector #41

JackCA opened this issue Dec 21, 2019 · 4 comments

Comments

@JackCA
Copy link

JackCA commented Dec 21, 2019

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?

@dai-shi
Copy link
Owner

dai-shi commented Dec 22, 2019

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?

@dai-shi
Copy link
Owner

dai-shi commented Dec 22, 2019

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);

@JackCA
Copy link
Author

JackCA commented Dec 22, 2019

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.

@dai-shi
Copy link
Owner

dai-shi commented Dec 23, 2019

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

@dai-shi dai-shi closed this as completed Dec 23, 2019
@dai-shi dai-shi changed the title Antipattern to recommend useTrackedSelector Discussion: Antipattern to recommend useTrackedSelector Dec 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants