RizkiDPrast
RizkiDPrast

Reputation: 1725

Prevent a method to be called multiple times in React JS

I am trying to experiment with reactjs multiple fields component by implementing Thinking in React but unable to figure out why method in the children called multiple times here.

var i = 0;
class SearchBar extends Component {
  constructor(props) {
    super(props);
    this.handleChange = this.handleChange.bind(this);
  }

  handleChange(key) {
    //this method is called multiple times during Render()
    console.log(i++);
    return function(e) {
      var value = key !== 'inStockOnly' ? e.target.value : e.target.checked;
      this.props.onUserInput(
        key,
        value
      );
    }.bind(this);
  }

  render() {    
    return (
      <form>
        <input type="search" value={this.props.filterText} placeholder="Search ..." ref={(input) => {input.focus();}} onChange={this.handleChange('filterText')} />
        <p>
        <input type="checkbox" checked={this.props.inStockOnly} onChange={this.handleChange('inStockOnly')} />
        {' '}
        Only show products in stock
        </p>
      </form>
    );
  }
}

class FilterableProducts extends Component {
constructor(props) {
  super(props);
  this.state = {
    filterText: '',
    inStockOnly: false
  };
  this.handleInput = this.handleInput.bind(this);
}
handleInput(key, value) {
/*var state = {};
  state[key] = value;  
  console.log(state);
  this.setState(state);*/

  //above is what I am trying to do later on but still stuck on why setState does not work
  //test
  this.setState({
    filterText: 'foo',
    inStockOnly: true
  });
}
  render() {
    return (      
      <SearchBar filterText={this.state.filterText} inStockOnly={this.state.inStockOnly} onUserInput={this.handleInput} />            
    );
  }
}
Update:

//updated handleChanged
handleChange(key, e) {
      var value = key !== 'inStockOnly' ? e.target.value : e.target.checked;
      this.props.onUserInput(
        key,
        value
      );
}

//updated handleInput
handleInput(key, value) {
  //var state = {};
  //state[key] = value;
  //this.setState(state);
  //console.log(state);
  //above is what I am trying to do
  //test
  this.setState({
    filterText: 'g',
    inStockOnly: true
  });
}
<!--i changed the event listener to this
  now its not called directly
-->
<input type="search" value={this.props.filterText} placeholder="Search ..." ref={(input) => {input.focus();}} onChange={(e)=>this.handleChange('filterText', e)} />

Any advice appreciated

Upvotes: 0

Views: 7158

Answers (2)

RizkiDPrast
RizkiDPrast

Reputation: 1725

just found out why my 'setState' "Not working". It is the because of unproper use of the ref attribute. By removing ref={(input) => {input.focus();}}, the code will works. And in my case I can change it to ref={(input) => if(input === null) return; {input.focus();}} will keep the focus function. It is because ref function is called twice: whenMount and didMount

so fully functional method will be:

class SearchBar extends Component {
  constructor(props) {
    super(props);
    this.handleChange = this.handleChange.bind(this);
  }

  handleChange(key, e) {
      var value = key !== 'inStockOnly' ? e.target.value : e.target.checked;
      this.props.onUserInput(
        key,
        value
      );
  }

  render() {
    return (
      <form>
        <input type="search" value={this.props.filterText} placeholder="Search ..." ref={(input) => {if(input === null) return;input.focus();}} onChange={(e)=>this.handleChange('filterText', e)} />
        <p>
        <input type="checkbox" checked={this.props.inStockOnly} onChange={(e)=>this.handleChange('inStockOnly', e)} />
        {' '}
        Only show products in stock
        </p>
      </form>
    );
  }
}

class FilterableProducts extends Component {
constructor(props) {
  super(props);
  this.state = {
    filterText: '',
    inStockOnly: false
  };
  this.handleInput = this.handleInput.bind(this);
}
handleInput(key, value) {
  var state = {};
  state[key] = value;
  this.setState(state);
}
  render() {
    return (      
      <SearchBar filterText={this.state.filterText} inStockOnly={this.state.inStockOnly} onUserInput={this.handleInput} />      
    );
  }
}

by using onChange={(e)=>this.handleChange('inStockOnly', e)} /> and onChange={(e)=>this.handleChange('filterText', e)} we can avoid using to much ref in multiple fields Component

Upvotes: 0

Martin Dawson
Martin Dawson

Reputation: 7656

Edit: That's correct, they are supposed to be called immediately because you are returning a function which binds to the onChange for when it fires. The inner function will only be called onChange. You will need to pass the key in though.

Second Edit:

handleInput(event) {
  this.setState({
    filterText: event.target.value,
    inStockOnly: true
  });
}
<input type="search" value={this.state.filterText} placeholder="Search ..." ref={(input) => {input.focus();}} onChange={this.handleInput} />

Original:

handleChange(key) {
    //this method is called multiple times during Render()
    console.log(i++);
    return function(key, event) {
      var value = key !== 'inStockOnly' ? event.target.value : event.target.checked;
      this.props.onUserInput(
        key,
        value
      );
    }.bind(this);
}

Event will be passed as the last parameter when it fires.

What isn't working about the setState? You are using it correctly in handleInput.

The commented out setState won't work though. You should only create state in the constructor and also do note though that the console.log() after you set the state may still be empty though because setState is asynchronous for performance and may not be updated until later.

var state = {};
state[key] = value;  
console.log(state);
this.setState(state);*/

You will want componentDidUpdate instead and perform the console.log in there or use the callback of setState, i.e this.setState({state[key]: value}, function() {console.log(this.state)}); which will always give the correct state.

Upvotes: 1

Related Questions