1

So the problem is as follows: I have a search function that get's an default value passed on from another place, the search works, but only when it gets a new input, hence if I'm passing "Dress" it wont call my api function before i change something in the input.

I've tried a bit of everything like setInitialState(), but without any noteworthy success.

As you can see I'm getting a onTermChange from my Searchbar that's passed to handleTermChange which then updates my products:[], but I need this.props.location.query to be the default search term, as this is the passed on variable.

  handleTermChange = (term)  => {
    const url = `http://localhost:3001/products?title=${term.replace(/\s/g, '+')}`;
    request.get(url, (err, res) =>  {
       this.setState({ products: res.body })
    });
  };

render () {
    return (
        <div className='col-md-12' style={{ margin: '0 auto' }}>
            <div className='row searchPageHeader' style={{ padding: '10px', backgroundColor: '#1ABC9C' }}/>
            <SideMenu />
            <SearchBar    onTermChange={this.handleTermChange}
                          defaultValue={this.props.location.query}/>
            <ProductList  products={this.state.products}
                          onProductSelect={selectedProduct => this.openModal(selectedProduct)}/>
            <ProductModal modalIsOpen={this.state.modalIsOpen}
                          selectedProduct={this.state.selectedProduct}
                          onRequestClose={ () => this.closeModal() }/>
            <Footer />
        </div>
       );
}
Sjohansen
  • 35
  • 6
  • So you want to automatically execute a search with the `location.query` value when the component mounts? – Chris May 11 '17 at 08:18
  • Yes, I want to automatically set my _products_ equal to the string that gets passed through _location.query_ so my location query should basically run through handleTermChange and update my productList – Sjohansen May 11 '17 at 08:36

1 Answers1

1

I would personally just do the same logic in componentDidMount(), like this:

componentDidMount () {
  const url = `http://localhost:3001/products?title=${this.props.location.query}`;
  request.get(url, (err, res) =>  {
    this.setState({ products: res.body })
  });
}

Note that since you are doing an asynchronous call products won't be populated from the API result until a moment after the component is mounted. Make sure you initialize products in initialState (I assume this returns an array, so initialize it as an empty array).


Opinion: Since you are following the event handler naming conventions (i.e onX followed by handleX) I would avoid calling handleTermChange() inside componentDidMount() because the function name suggests it's bound to an event listener. So calling it directly is just bad practice in my opinion. So if you'd rather call a function in here, rather than writing out the logic like I did above, I would do the following:

componentDidMount() {
  this.changeTerm(this.props.location.query);
}

changeTerm = (term)  => {
  const url = `http://localhost:3001/products?title=${term.replace(/\s/g, '+')}`;
  request.get(url, (err, res) =>  {
    this.setState({ products: res.body })
  });
};

handleTermChange = (term) => {
  this.changeTerm(term);
}

Your render() remains unchanged. Maybe a stretch, but I prefer it this way.

Chris
  • 57,622
  • 19
  • 111
  • 137
  • I'm getting a "term.replace is not a function" error when doing what you proposed – Sjohansen May 11 '17 at 09:10
  • @Sjohansen, that's probably because `term` isn't a string. Double-check you are passing `this.props.location.query`. Console it in your `render` to check. – Chris May 11 '17 at 09:12
  • 1
    My mistake, my _this.props.location.query_ returned an object with a string _q_, so when I changed that, it started working. Thank you! – Sjohansen May 11 '17 at 09:18
  • Just a quick best-practice question: would it make sense to move most of function calls into my search function instead of having them in my routes? – Sjohansen May 11 '17 at 09:20
  • Hard to answer. What functions are they? – Chris May 11 '17 at 09:21
  • The changeTerm, handleTermChange etc. – Sjohansen May 11 '17 at 09:25
  • I just noticed that I'm also getting an "Warning: setState(...): Can only update a mounted or mounting component. This usually means you called setState() on an unmounted component. This is a no-op. Please check the code for the Search component." – Sjohansen May 11 '17 at 09:30
  • By routes I assume you mean your views which contain the Searchbar. The best practice to try to create stateless components and have their parent or even grandparent manage their state. Ask yourself this: "*Does this component need to know its **own** state or should the parent know that?*". In my mind it makes more sense to have your view know what you searched for, rather than the Searchbar itself. The searchbar only accepts some text - how this text is managed is not relevant to it -- so pass it to your parent instead. Also, see [this answer](http://stackoverflow.com/questions/37170809/). – Chris May 11 '17 at 09:31
  • @Sjohansen, you probably searched for something and left the view before your API could respond to your query. This would execute the callback you have which tries to update the state of an unmounted component. – Chris May 11 '17 at 09:33
  • A sincere thanks for all the advice! I seem to have figured everything out, and got a little bit smarter as well! – Sjohansen May 11 '17 at 09:40
  • @Sjohansen, glad I could help. Please consider upvoting the posts you found helpful - it increases visibility for future readers that may face similar issues. – Chris May 11 '17 at 09:48
  • my reputation is sadly still too low for that. However, will do in the future of course! – Sjohansen May 11 '17 at 10:05