1

So I have a button that lives in a container and uses a callback on it's onClick event to update state in the container, fairly basic stuff. But now I want that container to let a different child know that the button was clicked so it can trigger the appropriate response. (and it should only let the child know once so the response isn't being triggered a million times)

The way I solved this looks and feels and quacks like a code smell, so I thought I'd ask you guys if there is a better way to do it. Here is what I did:

class myContainer extend Component {
   constructor(){
   super()
   state= { triggered: false }
   }
   componentWillUpdate(nextProps, nextState){
      this.hasTriggered = this.state.triggered !== nextState.triggered
   }

   triggerResponse = () => this.setState({...this.state, !this.state.triggered})

   render(){
      return (
         <myButton onClick={triggerResponse}/>
         <myComponent hasTriggered={this.hasTriggered}/>
      )
   }
}

Now this seems to work perfectly fine, and maybe this is what I should do, but it just feels like there has to be a neater way of sending a simple message of "I have been clicked" to a component in the same container. One major red flag for me is that "triggered" is a boolean, but it doesn't matter if it is true or false, so if triggered is false, it means nothing, all that matters if it was the other boolean last round. This seems like a violation of good practices to me.

*Summary: What I'm looking for is a snappy way to give state a value for just one update cycle and then go back to null or false without having to update it again. Or a different way to get the same result.

Odin Thorsen
  • 269
  • 3
  • 11
  • 2
    It's not an answer to your question, but there are two issues (one major) with `triggerReponse`'s `setState` call: 1. There's no need for `...this.state`, what you pass `setState` is a *partial* update, not a full one. 2. Much more importantly: Any time you're updating state based on state, you **must** use the callback version of `setState`. So that function should be: `triggerResponse = () => this.setState(prevState => ({triggered: !prevState.triggered}));` More about why [in the docs](https://reactjs.org/docs/state-and-lifecycle.html#state-updates-may-be-asynchronous). – T.J. Crowder Feb 28 '18 at 06:56
  • 1
    @T.J.Crowder Thanks! My favorite part about asking questions on SO is getting nuggets of good practice knowledge like these :) – Odin Thorsen Feb 28 '18 at 07:05
  • @OdinThorsen, You might check this question, https://stackoverflow.com/questions/46594900/reactjs-lifting-state-up-vs-keeping-a-local-state/47349693#47349693 This willl help you structure your code better, and achieve what you want in a better way in addition to what T.J Crowder said – Shubham Khatri Feb 28 '18 at 08:57
  • @ShubhamKhatri I think I'm already following the advice in that response by lifting the state up to the common container, the problem is that I'm (mis?)using a boolean and performing a check to emit a simple event down. What I'm looking for is a snappy way to give state a value for just one update cycle and then go back to null or false without having to update it again. Or a different way to get the same result – Odin Thorsen Feb 28 '18 at 10:19

2 Answers2

0

I came up with 2 different yet unsatisfying answers:

class myContainer extend Component {
   constructor(){
   super()
   state= { hasTriggered: false }
   }
   shouldComponentUpdate(nextProps, nextState){
      return (!nextState.hasTriggered && this.state.hasTriggered)
   }

   componentDidUpdate(nextProps, nextState){
      if(nextState.hasTriggered)this.setState({hasTriggered: false})
   }

   triggerResponse = () => this.setState({hasTriggered: true})

   render(){
      return (
         <myButton onClick={triggerResponse}/>
         <myComponent hasTriggered={this.state.hasTriggered}/>
      )
   }
}

This is unsatisfying because it is a lot of code for a very simple button click. I am setting state, sending it down, resetting state and then ignoring the next render call all to accommodate a lousy button click. The good news is that I'm no longer misusing a boolean, but this is definitely too much code.

class myContainer extend Component {
   componentDidUpdate(){
      this.hasTriggered = false
   }

   triggerResponse = () => {
      this.hasTriggered = true
      this.forceUpdate()
}

   render(){
      return (
         <myButton onClick={triggerResponse}/>
         <myComponent hasTriggered={this.hasTriggered}/>
      )
   }
}

I find this method unsatisfying because I no longer have the shared state in state. Before I also did this by comparing new and old state and making a variable carry the result over to my component, but at least then I could look at my state and see that there is a state variable that has to do with this button. Now there is local state in my component that is in no way linked to the actual state, making it harder to keep track of.

Odin Thorsen
  • 269
  • 3
  • 11
0

After thinking about this all day I've come to the conclusion that @ShubhamKhatri was on the right track in his comment, I thought I was pulling up state to my container by using a callback and passing state down, but clearly there is too much logic being executed in my component if it's handling a click event. So my new rule is that in this kind of scenario you should just pull up whatever state you need to execute the onClick inside the container. If your dumb components are executing anything other than a callback it is a mistake.

The reason I was tempted to do the onClick logic in my presentational component is because I was using a third party library(d3) to handle the graphics and so I didn't consider that the state I wasn't pulling up was the d3 state, if I move that up, change it when the button is clicked and then pass it down to my component it works beautifully.

Now this means I need to import d3 in two places and I did have to write a bit more code than before, but I think the separation of concerns and the overall cleanliness of my code is well worth it. Also it made my child component a lot easier to maintain, so that's nice.

Odin Thorsen
  • 269
  • 3
  • 11