rum icon indicating copy to clipboard operation
rum copied to clipboard

Question about state in mixins

Open narma opened this issue 11 years ago • 1 comments

In some cases rum\react is loosing non-default values in state between :did-mount and :will-unmount hooks in mixin. It doesn't happen if we manually transfer our state in :transfer-state hook.

The following example describes the issue https://gist.github.com/narma/46a990def0e1c6f279a2 In this code errors appear if we use dynamic count of children or we change parent elements.

Errors don't happen if we use constant count of children and don't touch parent elements because :will-unmount doesn't fire at all.

Is this expected and normal behaviour or is this a bug?

narma avatar Jan 16 '15 16:01 narma

Well, I know about this behavior, but not sure if it counts as a bug or a normal thing.

Here’s what happens:

  • page is a top-level component, and on re-render it creates new instances of widget.
  • we have now old widget instance and new widget instance. They both are legit, have their own state, etc
  • React tries to diff and decide whether it needs to re-render anything. It does so by comparing old instances with new ones. It basically just calls shouldComponentUpdate(old.props, new.props)
  • Old and new components are of the same class. React decides that it can save a re-rendering. So intead of going through full lifecycle, calling old.unmount, new.mount, it just calls old.componentWillReceiveProps(new.props) and keeps old instance. mount/unmount are not called at all

It’s now a design decision for Rum: assuming we have new component to replace old one of the same class, how do we handle state transition?

In some cases we don’t want to lose old state: e.g. (widget 1) → (widget 1). Sometimes we do want for state to be replaced: e.g. (widget 1) → (widget 7).

We cannot just use new state because it’s not fully initialized yet. Component should go through :will-mount and :did-mount loop first in order to get to proper state.

We cannot just keep old one because in cases like (widget 1) → (widget 7) we have to get state updated.

Raw React solves this by keeping old state by default + calling componentWillReceiveProps as a chance to update/replace old state manually.

Right now Rum solves it by always replacing old state with new one and providing transfer-state callback to move whatever you need to move from old one to new one.

Both approaches have a downside: we have two code pathes to generate component’s state. In case of Rum, it’s init + will-mount + did-mount and init + transfer-state. In case of React, it’s getInitialState + mount callbacks and further occasional updates from componentWillReceiveProps.

Both approaches lead to unexpected behaviors, subtle bugs when state gets lost (e.g. register listener without de-registering it, leading to listeners leak) and a lot of frustration.

I’m still deciding on what’s the best way to solve this is, so expect that this behavior will most probably change. Ideally I would prefer a solution where there’s always single path to component’s state. It should drastically simplify things. Suggestions welcome!

tonsky avatar Jan 17 '15 06:01 tonsky