facebookarchive / redux-react-hook

React Hook for accessing state and dispatch from a Redux store
MIT License
2.16k stars 103 forks source link

another crazy thought: reimplementation of Redux reselect.. built on top of this library #80

Closed warren-bank closed 4 years ago

warren-bank commented 4 years ago

Hi.. it's me again :)

This issue is unrelated to my previous issue #78.. where I introduced a little library that is a light-weight wrapper around the redux-react-hook library to add an enhancement to useDispatch().

This issue pertains to another enhancement that I added to that library, which is a function called createReduxSelector which aims to provide a (very nearly) drop-in replacement for Redux reselect.

Links to learn more..

I just wanted to let you know about it..

warren-bank commented 4 years ago

If you don't mind me picking your brain.. I'm on the fence about one topic that may have a slight impact on efficiency.. and I'm curious your thoughts.

Boiling the scenario down as much as possible.. here is the situation:

If a custom equality function (ex: shallow, deep, etc) was to be introduced to improve efficiency, would it be better to:

  1. apply it to useMappedState.. to prevent the component from re-rendering unnecessarily (ie: every time the global Redux state changes)
  2. apply it to the result of the function that depends on all the useMappedState output.. to memoize this result
  3. apply it to both (1) and (2)

currently:

Specifically:

in useMappedState:

https://github.com/facebookincubator/redux-react-hook/blob/5275dffd8defce004afd6ea5de3c9e30a0f987a1/src/create.ts#L137-L139


I'm debating changing the shape of the options Object to maybe something like:

{
  equality: {
    forceUpdate: [],
    recalculate: ''
  }
}

where:

shortcuts:

but.. I'm not entirely sure if this is the best strategy

any thoughts?

warren-bank commented 4 years ago

update: I just made the proposed change. Ultimately, the default behavior is unchanged. This just adds the ability to precisely configure all of the equality functions.. to fine tune (a) when mutation to Redux state should trigger the React component to re-render, and (b) how the the Redux selector is memoized. ..Always good to have knobs and levers for when you need them.

ianobermiller commented 4 years ago

That looks interesting, I'm having a bit of trouble following what it is for, though. I see a lot of hooks being called conditionally which is usually not good, but if it is the same on every call it might be fine. What does this library buy you over just using reselect directly?

I'd like to keep this library (redux-react-hook) lean, so this should be a separate module. I'm going to close this issue, but we can have further discussion here.

warren-bank commented 4 years ago

I totally agree with your concern about hooks being called in conditional blocks. While this appears to violate the 1st rule of hooks, this particular usage is safe under the implied contract that:

As for my motivation to experiment with a re-implementation of this library:

Since the purpose of this library is to provide good integration between Redux and React, these 2 missing ingredients (taken together) felt like a good candidate for inclusion. The useMappedState() method provides a very easy path to do so, since it already does most of the heavy lifting.

update 1 (correction):

update 2 (live examples @ codesandbox):

  1. no-frills (same as above):
  2. adds counters to track when functions execute (w/ interval timer to force a render every 2.5 secs):
  3. adds Redux state w/ corresponding input-selector (which derives a "debounced" counter value that changes once for every 3x the component renders)
  4. adds performance optimizations to reduce the frequency of component renders, such that it only occurs after the Redux selector returns a new value
warren-bank commented 4 years ago

To be entirely honest, I only glanced at the recommended methodology because it strikes me as very overly complicated.

If that methodology is feasible, then I would challenge you to fork example # 4 and rewrite it (using only redux-react-hook and optionally reselect) to produce the same output.

I'm honestly curious what that solution would look like, and how well it would scale to having more than only one input-selector that derives its value from Redux state.. as this example is a fairly trivial use-case.

update 1:

ok.. I took a closer look, and now I see what's happening in the recommended methodology. I'm going to accept my own challenge, and will post the link to a working example ~(..soon)~.

update 2: