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

Proposal: Add trivial dispatch mapping functionality #2

Open
raunofreiberg opened this issue Nov 10, 2018 · 2 comments
Open

Proposal: Add trivial dispatch mapping functionality #2

raunofreiberg opened this issue Nov 10, 2018 · 2 comments

Comments

@raunofreiberg
Copy link
Contributor

raunofreiberg commented Nov 10, 2018

Loving this library and it's simplicity so far. I'm just wondering why have you decided to expose dispatch directly instead of adding some trivial mapping of dispatch.

What I would love to see is:

  • useSubstate takes a secondary argument actions which is either a object or a function
  • Based on the type, call the actions with dispatch or simply bind the actions.
const mappedActions =
    typeof actions === "function"
      ? actions(store.dispatch)
      : bindActionCreators(actions, store.dispatch);
  • Return it as return [substate, mappedActions];

Would love to hear your feedback on this :-) This doesn't seem to complicate the implementation that much and just allows you to write out your actions directly in the hook itself.

E.g:

const [substate, actions] = useSubstate(
    state => ({ count: state.count }),
    dispatch => ({
      increment: () => dispatch(increment()),
      decrement: () => dispatch(decrement()),
    })
  );

or

const [substate, actions] = useSubstate(
    state => ({ count: state.count }),
    { increment, decrement }
  );
@raunofreiberg raunofreiberg changed the title Proposal: Add trivial dispatch mapping functionality Proposal: Add trivial dispatch mapping functionality Nov 10, 2018
@philipp-spiess
Copy link
Owner

Hey Rauno!

I don't think that action creator are a big improvement given that we would need to create them in the same function where we'd use them. For React Redux, those make more sense since the component itself does not need to be aware of Redux at all and can just call arbitrary props. However with this hook, the component is already aware that Redux is used so I don't think you would save a lot when creating action creators.

You can of course useCallback to create action creators regardless (see this example at the redux-react-hook package), or create a custom hook on top of useSubstate. Another alternative would be to use react-use-redux.

@raunofreiberg
Copy link
Contributor Author

raunofreiberg commented Nov 19, 2018

I'd think that the separation of concerns still applies in this case, regardless of the implementation being a hook rather than a HOC.

I still feel like when comparing the following two examples, the first one makes much more sense when thinking about where certain actions from. Especially if hooks are designed to sort of be called on the top level, I think having a proper mapping of dispatch to actions would really provide more encapsulated code.

Also, I'm not sure if there is a performance difference between declaring the action creators inside the hook vs. inline in the component?

function Counter() {
    const [substate, actions] = useSubstate(
        state => ({ count: state.count }),
        { increment, decrement }
    );

    return (
        <div>
            <h1>{substate.count}</h1>
            <button onClick={actions.increment}>+</button>
            <button onClick={actions.decrement}>-</button>
        </div>
    );
}
function Counter() {
    const [substate, dispatch] = useSubstate(
        state => ({ count: state.count }),
    );
    
    return (
        <div>
            <h1>{substate.count}</h1>
            <button onClick={() => dispatch(increment())}>+</button>
            <button onClick={() => dispatch(decrement())}>-</button>
        </div>
    );
}

Thanks!

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