Skip to content
This repository has been archived by the owner on Sep 10, 2022. It is now read-only.

[HOC suggestion] withToggle #244

Closed
jtadmor opened this issue Sep 15, 2016 · 8 comments
Closed

[HOC suggestion] withToggle #244

jtadmor opened this issue Sep 15, 2016 · 8 comments

Comments

@jtadmor
Copy link

jtadmor commented Sep 15, 2016

Hi,

A common use case for withState is a simple toggle. Right now, a combination of withState and mapProps needs to be used for this:

withState('key', 'toggleKey', true),
mapProps(({ ...rest, key, toggleKey }) => ({ ..rest, key, toggleKey: () => toggleKey(!key) })

or the calling Component needs to explicitly include the new state:

<Button onClick={() => toggleKey(!key) />

Neither of these is a huge hassle or roadblock, but it might be nice to encapsulate this common pattern by including a withStateToggle HOC that accomplishes this precise pattern:

withStateToggle('key', 'toggleKey', true)

<Button onClick={toggleKey} />

Happy to write a PR.

@istarkov
Copy link
Contributor

Hi ;-),
this is a common misconception that some common task for you is common for all ;-)

For example I use recompose since it first releases, and I never used some kind of toggle because of controllable nature of all components. So every component with 'toggle' state always pass it's new value in onChange handler.

So in my common ;-) case there is no need to use toggle at all.

I wrote about in some other issues, it's easier to think about recompose as about a base blocks for your own hocs. This will allow to make recompose API surface as small as possible.

It's so easy to compose multiple hocs and reuse everywhere in your code.

export default (keyName, keySetterName, initialValue) => compose(
  withState(keyName, keySetterName, initialValue),
  ...blabla
)

@jtadmor
Copy link
Author

jtadmor commented Sep 15, 2016

Thanks.

I was mistaken earlier - I just checked and mapProps does not create a separate component when used after withState.

(for clarity, my comment:
As with my other PR, I'll just point out that composing building blocks is great, but having to compose multiple building blocks for a common (this would clearly depend on whether other users wanted this in the lib or not) use case where there are potential hits to creating multiple components is unfortunate.
)

@istarkov
Copy link
Contributor

building blocks for a common

You are again talking about common ;-)
toggle is not common in the react world.
In jquery - yes,
In React - no.
I wrote above about why it's not so common as you think.

use case where there are potential hits to creating multiple components is unfortunate

Please be more clear, what use case are you talking about?

@jtadmor
Copy link
Author

jtadmor commented Sep 15, 2016

Never mind, editing my post.

@ntkoso
Copy link

ntkoso commented Sep 15, 2016

Personally i create separate modules like ramda-addons, recompose-addons in src/modules and add this directory to webpack resolve.modules config. That way there is no difference between 3rd party and my local imports.

import { withHandlers } from 'recompose';
import { withStateToggle } from 'recompose-addons';

@timkindberg
Copy link
Contributor

I am curious if @istarkov can elaborate on his approach with some code examples. He said:

So every component with 'toggle' state always pass it's new value in onChange handler.

What does that look like?

@wuct
Copy link
Contributor

wuct commented Feb 23, 2017

@timkindberg The problem of toggle is that it's not a pure function and is easy to replace with pure approaches, like: setValue(!value)

@wuct
Copy link
Contributor

wuct commented Feb 28, 2017

I am going to close this issue because it's no longer active. Please feel free to reopen it if you have further input.

@wuct wuct closed this as completed Feb 28, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants