Add First Class Support for Immutable-js Records (or maybe Maps) in setState?

[sebmarkbage]

Seems like this is a common use case for replaceState which we would like to deprecate. #3236

Not quite sure how this would work with Records and mixins since they're fixed type.

cc @leebyron

[samwgoldman]

Your reasons to remove replaceState are reasonable to me: difficulty adding new keys, clobbering mixin setStates.

Without replaceState, our only choice to transition immutable data would be to have the record/map on a property off state, e.g. state.data.

If the desire is to allow this.state to be something other than an extensible object, then I just don't see how it would be possible to guard against those two pitfalls in all cases.

The latter pitfall (mixins) seems more troublesome, but would not be an issue with ES6 classes, so maybe the solution is to allow non-object component state, but only with classes?

[dignifiedquire]

How about allowing to give a custom implementation of how setState handles the actual work, so for example one would write

class MyComponent extends Component {
    getInitialState() {
        return Immutable.Map({ name: 'World' })
    },

    applyState(oldState, partialNewState) {
        // this is a very simple implementation, we could do much more 
        // checks to ensure nothing breaks
        return oldState.mergeDeep(partialNewState)
    },

    _handleAction(data) {
         // Use it with another immutable object
         this.setState(Immutable.Map({ email: 'hello@world.com' }))

         // Use it with a plain JS object, as Map.mergeDeep handles plain objects as well
         // but we still ensure that the state object is immutable
         this.setState({ data: data })
    }
}

applyState would be used by React to do the actual updates if setState gets called.

This way there is no need to support specific libraries or anything like that, but everyone who wants to use immutable state can easily do so.

Another benefit is that as we define this method on a per component base, we can mix and match, as well as use it in libraries without the user knowing about this.

In the mixin case this should also as long as the mixin uses setState and calls it after the custom applyState method is activated.

(applyState is not the name I'm suggestion, it's just a placeholder to start a discussion)

What do you guys think?

[WishCow]

I can't comment on the implementation, I just want to +1 this, my app doesn't work in 0.13 (#3418), and I'd be happy if I didn't have to put my state data under some dummy key.

[Zenwolf]

I agree, having a form of support for immutable state data structures is important to me as well.

[leebyron]

Seems like the core incongruence is between using stateful mixins and having control of your component's state. Using stateful mixins means giving up some of this control to your mixins.

The practice of using replaceState() alongside Immutable.js came with the caveat that you couldn't use stateful mixins.

I see a few ways out of this:

1. mixins are barred access to setState, they cannot be stateful. This is possibly an unfortunate limitation.
2. mixins have their own state, distinct from the component they're mixed into. This makes them even more "special" than they already are (which is a bit more special than just a bunch of methods to be mixed into a class prototype). This would encourage data isolation between different mixins and between mixins and components, so each maintains control. This adds weight to the React abstraction, also unfortunate.
3. Add some duck-type interface for state to allow overriding the state "merge" strategy. Here's a very very rough example of what this could look like:

function merge<T>(prevState: T, stateDeltaObj: Object): T {
  var mergeStrategy = prevState['@@reactMergeStrategy'];
  if (mergeStrategy) {
    return mergeStrategy.call(prevState, stateDeltaObj);
  }
  return Object.assign({}, prevState, stateDeltaObj);
}

This would allow for initial state to be any kind of object that supports this interface, and we could keep using the existing setState API.

---

I'm actually leaning in favor of option 1 here, thinking that perhaps stateful mixins would be better off as wrapping components. Component composition could provide the functionality that stateful mixins can be used for today. I'm leaning in favor of this because it's the only option of the three that actually simplifies rather that adds complexity to React.

In the case that we go with option 1, I would propose also adding an API that's parallel to setState(), perhaps called updateState() which always takes a function and that function always returns an entirely new state object. This kind of API would put the most amount of control back in the hands of developers using React.

This would allow you to turn code that looks like this:

class ClickCounter extends React.Component {
  getInitialState() {
    return { 
      data: Immutable.Map({ count: 5 }) 
    };
  }

  updateCount() {
    this.setState(prev => ({ data: prev.data.update('count', d => d + 1) }));
  }

  render() {
    return <div onClick={this.updateCount}>{this.state.data.get('count')}</div>;
  }
}

Into code that looks like this, one layer of indirection removed from state access.

class ClickCounter extends React.Component {
  getInitialState() {
    return Immutable.Map({ count: 5 });
  }

  updateCount() {
    this.updateState(prev => prev.update('count', d => d + 1));
  }

  render() {
    return <div onClick={this.updateCount}>{this.state.get('count')}</div>;
  }
}

[sebmarkbage]

The concern with having two different update functions was that we were hoping to go into the direction where state is always updated through returning values.

https://github.com/reactjs/react-future/blob/master/07%20-%20Returning%20State/02%20-%20Module%20Pattern.js#L17

I'm not sure that direction is still to be considered "the current thinking" though.

This proposal also seems half-way to me. If you're using Immutable.js in this way, it seems like what you really want is an Om-like cursor like:

this.state.update('count', d => d + 1);
// or
this.updateState('count', d => d + 1);

Why call an "update" twice? I feel like this is a case where pluggability leads to worse results for everyone and a more opinionated solution is preferable.

[sebmarkbage]

There are a few differences going on here:

1. Immutability. Not really relevant.
2. The update helper is attached to the object itself.
3. Map vs. Record that contains a Map.

You could imagine a mutable Map instead of a Record so that concern exists for both.

this.state.data.set('key', value);

is longer than

this.state.set('key', value);

So it seems to me that the issue here is the convenience of using the updater. If the top level object was an Immutable.Record which was auto-merged, you could do:

this.setState(this.state.updateIn(['data', 'key'], value)));

[jeffutter]

Has there been any movement on this?

[ikhilko]

Hi, any progress on it? This issue is still actual...

[clouddra]

Any updates on this issue?

[necolas]

This is a very old issue and I don't think we have any plans to do this anymore