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

Fix undefined children does not update children in state #50

Merged
merged 2 commits into from
Aug 11, 2015
Merged

Fix undefined children does not update children in state #50

merged 2 commits into from
Aug 11, 2015

Conversation

istarkov
Copy link
Contributor

If at first render you have children props, and at second not
(this.props does not contain children element),
this.state contains previous version of children.

@istarkov
Copy link
Contributor Author

And it look's like the problem is deeper
for any component props which is gone
you can't use setState because of it's merge nature,
it's need something like replaceState

@gaearon
Copy link
Contributor

gaearon commented Aug 11, 2015

Can you add a failing test case instead? I don't understand the problem yet.

@istarkov
Copy link
Contributor Author

Ok i'll add, the problem is that setState merges newState with old state.
And if at first rendering step you have some prop, and at next rendering step this prop is not exists,
connector state still contains prop from first rendering.
As example

render () {
  const obj = this.props.a ? {someProp: 1} : {};
  <ReduxConnectedComponent {...obj} />
}

if this.props.a was true, and becomes false, ReduxConnectedComponent still contains this.props.someProp equal to 1

@istarkov
Copy link
Contributor Author

So recomputeState must be like

        recomputeState(props = this.props) {
          const nextState = this.computeNextState(props);
          if (!shallowEqual(nextState, this.state)) {
            this.setState({
              ...Object.keys(this.state)
                .reduce((r, k) => ({
                  [k]: undefined
                }), {}),
              ...nextState,
            });
          }
        }

@istarkov
Copy link
Contributor Author

I've add failing test, (without @connect it works as expected)

gaearon added a commit that referenced this pull request Aug 11, 2015
Fix undefined children does not update children in state
@gaearon gaearon merged commit 1edb697 into reduxjs:master Aug 11, 2015
@gaearon
Copy link
Contributor

gaearon commented Aug 11, 2015

I got it now, thanks a lot!

@gaearon
Copy link
Contributor

gaearon commented Aug 11, 2015

This is out in 0.8.1, if you see any use cases that aren't covered yet, feel free to add more tests ;-)

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

Successfully merging this pull request may close these issues.

2 participants