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

Not working component update on state change. #57

Closed
jsifalda opened this Issue Jun 7, 2015 · 6 comments

Comments

3 participants
@jsifalda

jsifalda commented Jun 7, 2015

Few things:
First one: Why this? https://github.com/gaearon/redux/blob/master/src/components/Connector.js#L19 - not sure whether "flux" lib should cream about this part.
Second one: due to custom method shouldComponentUpdate "rerendering" of component does not work.

Look at my code:
Inside container component:

<Connector select={select}>
              {({ userFooter, dispatcher }) =>

                <UserFooter userFooter={userFooter} {...bindActions(UserActions, dispatcher)} />
              }
            </Connector>

select method:

let select = (state) => {
  return {
    userFooter: state.userFooter
  }
}

Store:

const initialState = {
  isAuthenticated: false,
  email: ''
}

let setAuthenticated = (state) => {

  state.isAuthenticated = false
  state.email = ''

  return state
}

export default createStore(initialState, {

  [LOGIN_SUCCESS]: (state, action) => {

    state.isAuthenticated = true
    state.email = action.email

    return state

  },

  [LOGIN_FAILURE]: (state, action) => {
    return setAuthenticated(state)
  },

  [LOGOUT]: (state, action) => {
    return setAuthenticated(state)
  }
})

Then state.isAuthenticated will change to false. But Connector component does not render component "UserFooter" inside.

UserFooter component:

export default (React) => {

  let types = React.PropTypes

  let UserFooter = (props) => {

    console.log('props', props)

    return {

      __proto__: React.Component.prototype,
      props,

      logout (e) {

        e.preventDefault()

        this.props.logout()

      },

      render () {

        let {isAuthenticated, email} = this.props.userFooter

        console.log('render', isAuthenticated)

        return (
          <div>

            {isAuthenticated ? (
              <p>
                Logged as : {email}
                <br/>
                <a onClick={(e) => this.logout(e) }>Sign Out</a>
              </p>

            ) : (
              <Link to='login'>Sign In</Link>
            )}

          </div>
        )
      }
    }
  }

  UserFooter.propTypes = {
    // isAuthenticated: types.bool.isRequired,
    // email: types.string,
    userFooter: types.object.isRequired,
    logout: types.func.isRequired
  }

  return UserFooter

}

Last thing: Amazing work mate!

@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon Jun 7, 2015

Collaborator

I agree it's not very clear from the README, but Redux makes a hard assumption that you never mutate the state. Mutating the states defeats the purpose of Redux, and it's easy to avoid mutating it, so it's in fact a net good that the mutating code breaks early.

We should make it explicit in the docs.

Here's how your Store could look instead:

const initialState = {
  isAuthenticated: false,
  email: ''
}

let setAuthenticated = (state) => {
  return {
    isAuthenticated: false,
    ...state
  }
}

export default createStore(initialState, {

  [LOGIN_SUCCESS]: (state, action) => {
    return {
      isAuthenticated: true,
      email: action.email,
      ...state
    }
  },

  [LOGIN_FAILURE]: (state, action) => {
    return setAuthenticated(state)
  },

  [LOGOUT]: (state, action) => {
    return setAuthenticated(state)
  }
})

Does this solve your problem?

Collaborator

gaearon commented Jun 7, 2015

I agree it's not very clear from the README, but Redux makes a hard assumption that you never mutate the state. Mutating the states defeats the purpose of Redux, and it's easy to avoid mutating it, so it's in fact a net good that the mutating code breaks early.

We should make it explicit in the docs.

Here's how your Store could look instead:

const initialState = {
  isAuthenticated: false,
  email: ''
}

let setAuthenticated = (state) => {
  return {
    isAuthenticated: false,
    ...state
  }
}

export default createStore(initialState, {

  [LOGIN_SUCCESS]: (state, action) => {
    return {
      isAuthenticated: true,
      email: action.email,
      ...state
    }
  },

  [LOGIN_FAILURE]: (state, action) => {
    return setAuthenticated(state)
  },

  [LOGOUT]: (state, action) => {
    return setAuthenticated(state)
  }
})

Does this solve your problem?

@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon Jun 7, 2015

Collaborator

(Suggestion: maybe deep freeze the state atom in development mode?)

Collaborator

gaearon commented Jun 7, 2015

(Suggestion: maybe deep freeze the state atom in development mode?)

@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon Jun 7, 2015

Collaborator

I added this to FAQ:

https://github.com/gaearon/redux/blob/master/README.md#my-views-arent-updating

Let me know if I missed something!

Collaborator

gaearon commented Jun 7, 2015

I added this to FAQ:

https://github.com/gaearon/redux/blob/master/README.md#my-views-arent-updating

Let me know if I missed something!

@gaearon gaearon closed this Jun 7, 2015

@ooflorent

This comment has been minimized.

Show comment
Hide comment
@ooflorent

ooflorent Jun 7, 2015

Collaborator

I think I've heard that Facebook will freeze props and maybe state when NODE_ENV isn't production. You should do the same.

Collaborator

ooflorent commented Jun 7, 2015

I think I've heard that Facebook will freeze props and maybe state when NODE_ENV isn't production. You should do the same.

@ooflorent

This comment has been minimized.

Show comment
Hide comment
@ooflorent

ooflorent Jun 7, 2015

Collaborator

Another option is to clone the state in dev before injecting it to the component.

Collaborator

ooflorent commented Jun 7, 2015

Another option is to clone the state in dev before injecting it to the component.

@jsifalda

This comment has been minimized.

Show comment
Hide comment
@jsifalda

jsifalda Jun 7, 2015

Much better! Thank you very much! :-)

jsifalda commented Jun 7, 2015

Much better! Thank you very much! :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment