-
-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
withHooks HOC to decouple components from hooks #597
Comments
This is a really interesting proposal. I had to do something similar while ago for using some hooks within legacy class components, it's quite useful when you can't refactor your class components but you need to reuse a hook. That said, I don't know if this should be included on this package as it seems outside of its scope and maybe it makes more sense to have it in its own package. Let's see what @streamich thinks about this. |
Yes, this seems like an ideal use case, to make class components pseudo-compatible with hooks. I think along the same lines withHooks makes migrating a large class component to a functional component significantly simpler because it can be done incrementally. The component can remain a class component indefinitely, but bits and pieces can be pulled out as hooks. |
What about creating a component in between, instead of introducing HOC ( Modifying last part of OP's example: const EnhancedCounter = props => {
const [count, setCount] = useState(0);
return (
<Counter
count={count}
increment={() => setCount(prev => prev + 1)}
decrement={() => setCount(prev => prev - 1)}
/>
);
};
ReactDOM.render(<EnhancedCounter />, document.getElementById("root")); |
I would say that creating this intermediate component is almost equivalent, except i see two primary differences.
const withCounterApi = withHooks(() => {
const [count, setCount] = useState(0);
return {
count,
increment: () => setCount(prev => prev + 1),
decrement: () => setCount(prev => prev - 1)
};
});
const EnhancedComp1 = withCounterApi(Comp1)
const EnhancedComp2 = withCounterApi(Comp2)
const EnhancedCompN = withCounterApi(CompN) |
As this library deals with hooks, it feels like a generic utility to create a HOC out of a hook is something we should have. We actually already have a function that creates a render prop component out of a hook. I would name it When the hook returns an object, |
Guys could you please review the above PR and help me out with the correct |
Is your feature request related to a problem? Please describe.
With the introduction of the Hooks API in React 16.8, developers have been shifting away from Higher Order Components and Render Props patterns to handle cross-cutting concerns amongst React components. Prior to Hooks, recompose offered a robust library of HOC's with similar functionalities to the Hooks API.
Since then, the creator of recompose and the React team have been advising developers make the switch from recompose to Hooks as they feel Hooks solve all of the issues recompose aimed to solve, but avoid issues inherent to recompose such as excessive tree nesting.
However, Hooks are not perfect either. And there's been a lot of discussion around one issue in particular. The recommended way to implement Hooks is to couple them with the components that depend on them. By doing this, we complicate things such as render bail-outs and testing because now components bake-in hook logic and explicitly depend on these hooks as their data/logic providers. One of the nice things about recompose HOC's is that they inject similar functionality through props, meaning the components are not explicitly dependent on their data providers. This issue thread on the recompose repo discusses much of the motivation for a withHooks utility function.
Describe the solution you'd like
See demo
By placing all of our hook logic in a higher order component, we can pull hook logic out of our component and create an interface that will be passed as props to our wrapped component (the Counter in this case). At first glance,
mapHooksToProps
looks like a functional component, but it is not. It is a custom hook that aggregates other hooks and exposes an API from them.mapHooksToProps
is run on each render and returns an object that is provided as props to the wrapped component.Why do we want to decouple components from hooks?
It decouples components from the things that make them smart. Some examples of things that could make dumb components smart include hooks, redux, and good ol' parent components. By mapping hooks to props, we make it very easy to swap a dumb component's hook-powered 'brain' for a new 'brain', say a redux-powered 'brain'. To make this switch, we wouldn't have to touch the underlying component, we would just have to swap out
withHooks
with react-redux'sconnect
function and make sure we conformed to the pre-existing interface (that returned bymapHooksToProps
).Because we're deriving an API from hooks and injecting it via props, we can bail out of renders using existing mechanisms (shouldComponentUpdate, React.memo, React.PureComponent). Here's an example showing how we can bail-out of renders using
withHooks
.Testing is made easier because we can test components in isolation, without hooks baked in, and we can test our hook logic in isolation of our components. By pulling all stateful logic into
mapHooksToProps
and then injecting it via props, we have full control of our components via props, which makes testing super simple.It let's us use hooks with class components. We can wrap class components with hooks and inject hook-powered logic via props.
It makes prop overrides possible. In the case of our EnhancedCounter, we could override the count prop by doing
<EnhancedCounter count={10} />
. A real world example of hooks making things harder to override includes material-ui's new styling approach via hooks. Because classes are provided via hook and no longer via props, we would need extra code in our component to override classes via props with the new hook-based approach:(In this case
useStyles
optionally takes in the host's props as the first argument to allow for class overrides, but ignoring that to illustrate a point)The text was updated successfully, but these errors were encountered: