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

withState alternatives #308

Closed
istarkov opened this issue Jan 28, 2017 · 6 comments · Fixed by #421
Closed

withState alternatives #308

istarkov opened this issue Jan 28, 2017 · 6 comments · Fixed by #421

Comments

@istarkov
Copy link
Contributor

Very good looking idea here https://twitter.com/jordwalke/status/825305448542724099 which can be used instead or with withState.

some reasons why current implementation can possibly be changed

So creating an enhancer which will wrap setState with

updater = memoize(
  (stateUpdater, callback) => 
    (...args) => 
      this.setState(
        (state, props) => stateUpdater({ state, props },  ...args),
        callback
     )
)

will give us ability to use updater as in twit example

const onInputValueChange = 
  ({ props, state}, event: { target: { value: inputValue }}) => ({  inputValue })

const Component = ({ updater, inputValue }) => (
  <input value={inputValue} onChange={updater(onInputValueChange)} />
)

Having that updater is memoized it will give the same function reference in onChange handler on every rerender.

@wuct
Copy link
Contributor

wuct commented Jan 28, 2017

I believe Elm also use the similar API, and it looks promising to me.

@istarkov
Copy link
Contributor Author

Just have read this facebook/react#8854 and there is possibility that this will be done in React, see statefull functuonal components, so in partial cases like all enhancers are referentially transparent, this will work from scratch.

@wuct
Copy link
Contributor

wuct commented Feb 2, 2017

I am really looking forward to stateful functional component, but it will not land in 16 😞

@slightlytyler
Copy link

@istarkov are you suggesting a new HOC for the library? withValue?

@istarkov
Copy link
Contributor Author

@slightlytyler there are some problem with this HOC implementation, the same problem have guys with react bindings for reasonml.
The problem that updater should do an atomic updates using react syntax setState((state, props) => ...) and currently in React there's no opportunity for you to say "don't update state if it's not needed"
So every call to updater someHandlerwill cause a state refresh and rerender even if state will not be changed.
For now react bindings for reasonml uses non atomic updates. Which in some rare situations can cause a problems.

@timkindberg
Copy link
Contributor

Picking up the conversation from #321 (which would also solve for #243).

I love that you are looking ahead to future apis, and I think adding an alternative signature until then is not a bad thing. There's no reason to wait. Especially for such a simple, backwards compatible and non-controversial change. After reviewing this comment I've updated the proposal:

Add a new signature to withState that would be used when 1 or 2 args were passed. If 3 args are passed the existing functionality would be used. The new signature would be:

withState(
  initialState: Object | (props: Object) => Object,
  mapStateProps: ?(state: Object, setState: Function ) => Object
): HigherOrderComponent

Only initialState is required. mapStateToProps can be used to rename the defaults of state and setState but would not be required. Additionally calling setState would behave the same as the setState from a class component, where new state is merged with existing state, not overwriting.

Usage would look like:

import { withState } from 'recompose';

// setting initial state only
const enhance = withState({ red: 0, green: 0, blue: 0 });

// automatically get state and setState props
const RGBChooser = enhance(({ state, setState }) => 
  <div>
    <input type="text" value={ state.red } onChange={ (e) => setState({ red: e.target.value }) } />
    <input type="text" value={ state.green } onChange={ (e) => setState({ green: e.target.value }) } />
    <input type="text" value={ state.blue } onChange={ (e) => setState({ blue: e.target.value }) } /> 
    <div
      style={{
        display: block,
        width: 30,
        height: 30,
        backgroundColor: `rgb(${state.red},${state.green},${state.blue})`
      }}
    /> 
  </div>
);

If I submit a pull request for #321 (code, tests, docs) would it be accepted?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants