Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Question about state in mixins #11

Open
narma opened this issue Jan 16, 2015 · 1 comment
Open

Question about state in mixins #11

narma opened this issue Jan 16, 2015 · 1 comment

Comments

@narma
Copy link

narma commented Jan 16, 2015

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?

@tonsky
Copy link
Owner

tonsky commented Jan 17, 2015

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!

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

No branches or pull requests

2 participants