Recompose: [withState improvement]

[jtadmor]

Hi,

Love the lib / work, thanks for this.

One suggested improvement:

Currently, withState can only accept a single string arg for the stateKey and updater.
If I want multiple keys / updaters, I have to wrap with withState multiple times, which according to my React dev tools creates multiple HOC:

withState(withState(MyComponent))

Suggested alternate API signature (would not affect any existing calls):

withState(
  ['name', 'updateName', (props) => props.book.name],
  ['description', 'updateDescription', ''],
  ...
)

If this seems reasonable I'm happy to work on a PR

[istarkov]

You could always update multiple values now:

withState('obj', 'setObj', { name: '', description: ''})
....
withHandlers({
  changeDescriptionAndName: ({ setObj }) => ({ name, description }) => {
    setObj(o => ({...o, name, description }))
  },
  changeDescription: ({ setObj }) => ({ description }) => {
    setObj(o => ({...o, description }))
  }
})

So sorry, but I don't think that your PR will make something simpler or easier.

[jtadmor]

Respectfully, I think that approach has some downsides:

1. Harder to read and understand, both because there are multiple wrappers and because they are more abstract. What does setObj do?
2. My enhanced component now receives a prop of obj, which I don't want. It was supposed to receive name and description, so now I need another wrapper (mapProps).
3. More work for me as a developer ;)
4. Multiple wrappers. Possible Perf issues. Harder to read React devTools when I have withState(withHandlers(mapProps(MyComponent)))

So the question is whether that downside is worth adding the extra lines of code to the project / another export that needs to be unit tested.

[istarkov]

1 setObj is just an example, name it as you want
2,3 modern js allows to use this notation ({ obj: {name, description} }) it's not a big delta of job vs { name, description }
Do you really use devTools a lot? In a lot of components removal of few withStates does not affect mach the overall tree depth. BTW here I understand your pain even I does not use dev tools.

PS:
IMO design of withState should be changed somehow (I don't know how) to remove all that string names of property and method. The reason I have that current notation will prevent modern typesystems (flowtype, typescript) to automatically detect output props type.
So there is no need to extend current design in the possibly wrong direction.

[jtadmor]

Perhaps you feel you're being helpful, but your comments come across as condescending.

Rather than imply I don't know how to use "modern js" (or make it seem like users of this library who aren't using FlowType / es6 / babel are bad or wrong), can you just take it at face value that I do use devTools and that removing the withState wrappers will make my devTools experience better?

(Not to mention it will as a matter of fact require less calls to React.createElement, because each withState HOC currently creates a separate wrapper with stateValue key instead of setting state internally based on the key passed in.)

On top of that, I don't get the impression that you're trying to hear what I'm saying to begin with.

What I'm talking about is that you are asking me to change my component's prop signature into accepting { name, description } as a single prop.

It is natural to write:

BookDisplay = ({ name, description, ...other }) => (
  <div>
    <h1>{name}</h1>
    <p>{description}</p>
   // Render with other here
  </div>
)

It is NOT natural to write

BookDisplay = ({ obj, ...other }) => {
  const { name, description } = obj

  return (
    <div>
      <h1>{name}</h1>
      <p>{description}</p>
     // Render with other here
   </div>
 )
}

The problem here isn't the label "obj". The problem is that now my functional component needs to change its signature based on how its being enhanced. My component needs to know about the structure of the enhancers to know which props came from which object. That's not a good pattern.

But yeah, I can drop this and implement it myself as a separate lib, peace.

[istarkov]

Ok. I'll try to explain in other words.
Your suggestion here and in #244 is to replace composition with inclusion.
In most cases I prefer composition. Even this library name have compose word.

You could look here #182 and see that composition could be done in a way where just one upper level component will be created, but read this comment https://github.com/acdlite/recompose/pull/182#issuecomment-230546987

[threehams]

Typescript 2.1 now has type safety for string lookups, so that shouldn't be an issue once the typings are updated.

https://www.typescriptlang.org/docs/handbook/release-notes/typescript-2-1.html

[wuct]

Can be further discussed in #308. Closed now.