Clearing ES6 Stores doesn't restore the initial state

[moretti]

Hi, according to the docs, Store.clear should replace the state with the initial state when called.

This works fine in classic mode because Store.clear internally calls Store.getInitialState.
However in ES6 mode Store.getInitialState is not defined because the state is typically initialised in the class constructor and calling Store.clear simply sets the state to an empty object.

I ended up using getInitialState in my ES6 stores as temporary workaround, but it seems an anti-pattern.

Any thoughts on this?

[taion]

I'm actually using getInitialState in my ES6 stores as well, for exactly this reason. It doesn't seem awful - maybe we should capture the initial state for stores after registration, though.

[jhollingworth]

This is an annoying side effect of ES6 and something I'm not overly happy with. The issue with capturing the initial state is, if its mutable, you're just re-assigning the initial state to itself. Also, cloning the object has lots of issues (e.g. what happens if its a class?).

I concluded that Store#clear wasn't used that much (I mainly used it in tests which is no longer needed now we have no singleton) and it was better to have a simpler API that reflects React's. I'm happy to make getInitialState required in v0.11 if you think this is better?

[taion]

I would just _.cloneDeep(store.state) on registration and assign it somewhere. I think that might be a nice convenience.

[moretti]

I agree that Store.clear is not really useful in ES6 mode, I think it would be probably simpler to just
reinstantiate the class. The reason why I have this problem is because I'm invoking app.clearState to wipe out every store when users sign out.

[taion]

I thought Lodash's cloning function handled most cases with objects with non-trivial prototypes. Might be worth investigating - probably we need getInitialState to handle all edge cases, but if cloning the initial state is sufficient for most use cases, then we should do so.

[jhollingworth]

What about

this.state = {
  user: new User(123)
}

Which results in _.clone(this.state).user === this.state.user. Do you think this correct behaviour?

[taion]

Hmm, and cloneDeep doesn't preserve the constructor (i.e. you just get an object).

That said, isn't this going to generally break dehydration/rehydration anyway? I still think this should do the right thing in most cases - anybody who needs something special can implement getInitialState, just like they do with custom dehydration and rehydration methods.

[jimmed]

I'm with @moretti on this one - admittedly I'm heavily biased as I work with him.

Naturally, capturing the initial state and re-assigning it on clear isn't suitable due to the state possibly being mutable (or uncloneable). Unless I'm missing something, simply destroying and re-instantiating the store object is the only clear solution to this problem.

This would support both ES6 and ES5 implementations (class-based or using Marty.createStore) without suffering any state mutation issues, or serious performance hits.

I'm open to suggestions!

[taion]

@jimmed I see what you mean now. That makes a lot of sense.