Crazy question: Change state API?

[goatslacker]

Do people like managing state through the instance properties or should we adopt a React 0.13-like API where state is set on a state property?

Now:

class TodoStore {
  constructor() {
    this.todos = []
  }
}

React 0.13-like

class TodoStore {
  constructor() {
    this.state = { todos: [] }
  }
}

I initially went with the instance properties approach because back in the old days of writing JavaScript where we'd have data stores we would create them as a function and constructor and then keep the state as instance properties. I felt that just changing an instance property was familiar enough to everyone and had less boilerplate.

I'm considering this new change because of a few things:

• Parity with React.

  Although alt prides itself on not being tied down to React, lets face it, Flux and React are like Peanut Butter and Jelly. Having the same API would make things smoother.

• Private instance variables.

  Currently you can have your private instance variables, but you'll have to use semi-private Symbols or wait for TC-39 to add language features to add private state to a class. By having this change anything not specified in state is private which can be nice.

• Stores are automatically serializable so with state you can define what is serialized right from the start.

  This is a big reason. Say for example you're using something like firebase.

class TodoStore {
  constructor() {
    this.firebase = new Firebase()
    this.ref = this.firebase.child('accum')
    this.ref.on('value', () => 2)
  }
}

this will throw an error because these properties are not serializable.

The current solution? To either explicitly specify what is serializable via lifecycle methods, or use symbols to store that data. While this is nice and explicit I think we can remove this boilerplate entirely.

Cons of this change:

• It's a breaking change and that stinks.
• It makes the API very react-like.
• It is probably not the JS we will be writing in the future if JS does end up with private instance properties.

Proposal for migrating to this:

• Introduce warning in next release
• Change all examples to this new syntax
• Make this backwards compatible somehow
• Remove old behavior in 1.0?
• Write a script that will automatically migrate you from old to new.

[frederickfogerty]

Yes, I love it. I agree with all the points you have raised. Other Flux stores don't have this issue as they force users to implement their own public methods, whereas alt relies on getState. In other Flux stores thus you can have private and public properties without much effort. This would be the best way to do it in alt.

It's a yes from me.

[troutowicz]

👍

[srph]

* insert traditional thumbsup + sparkle * 👍 ✨

[alvinsng]

The only backwards compatible thing I think of is if this.state is undefined then use the instance properties over this.state

Sigh... oh breaking changes.... maybe think of other breaking changes at the same time so you can do a major version bump?

[spikebrehm]

Makes sense to me.

I also like the idea of adopting a getInitialState() method. IMO it's cleaner to have a method that returns a hash:

getInitialState() {
  return {
    count: 0,
    foo: 'bar',
  };
}

rather than setting this.state inside the constructor:

constructor() {
  this.state = {
    count: 0,
    foo: 'bar',
  };
}

I've been adopting this pattern in my Alt stores.

[mehcode]

Take a look at http://babeljs.io/blog/2015/03/31/5.0.0/

---

You can now set instance properties in a class:

class Store {
  state = { count: 0 };
}

[goatslacker]

@spikebrehm React 0.13 lets you set state directly in the constructor and you don't use getInitialState any more AFAIK.

@mehcode

• Is that for sure part of ES7?
• Won't that be part of the prototype?

Found the answer here: https://gist.github.com/jeffmo/054df782c05639da2adb It's a Stage 0 proposal so that's good but I wouldn't use it just yet since it may change drastically before it ships.

[ziflex]

Nice, but is 'backwards compatible' really needed ? If warnings and migration tool will be provided, this option is useless, in my opinion :)

[goatslacker]

Backwards compatible for the interim.

[jdlehman]

I don't have a strong preference with this change, though I think the ability to more easily have "private" instance variables regardless of how we do it is a good feature.

Despite my lack of a strong preference, I do think this brings up an even bigger issue. The issue is not just moving the data to a state property, but the fact that we are starting to make changes that are more React-oriented. This is not necessarily a bad thing, but one nice thing about alt and the Flux pattern in general, is that it is not dependent on React. It definitely makes sense to use with React, and I think most of us are using React with alt, but it could be used without it.

I am curious if anyone is using alt without React as well as people's thoughts about how much alt should cater to React. I would like to try to keep alt flexible such that it can be used without React (even if it's API ends up being similar to React), but I think it is a worthwhile discussion to figure out how much alt should assume.

[jareware]

I'm not as much against this change as I just like the current one better. Here's some pseudo-random thoughts this raised:

• React uses a method for setting state (setState()), and assigning directly to state isn't kosher. So this isn't equivalent, whether that's a pro or a con. :)
• I take it the arguments for privacy apply for stores accessing other stores during a dispatch? Because when querying the stores during rendering we already have nicely enforced privacy. I'm not sure we actually need it between stores during dispatch.
• The current API is dead simple. Can't get more POJO than just freely assigning stuff to properties.
• The current solution is type-checker friendly. I haven't gotten around to exploring this too much yet, but e.g. Flow just loves signatures like the simple POJO-like form that we currently have. As an added bonus, the read-only accessors can actually be defined in a separate class that extends the actual POJO-one, so you should be able to define that as the interface you use during render() and the other one during action dispatch. But I'm going off on a tangent... :)
• For better or worse, this makes it easier to sneak in transient state to the stores. The current mechanism for snapshots/rollbacks discourages this, and I think that's at least a very good default, as having two types of state (persistent and transient) in stores makes everything just a bit less simple. That said, the Firebase example is a good one...

Anyway, my 0.02 EUR. :)

[goatslacker]

Killing this in lieu of #173