2

How can we setState in componentDidMount hook within an argument?

My problem is the warning of React notify that:

Warning: Can't call setState (or forceUpdate) on an unmounted 
component. This is a no-op, but it indicates a memory leak in your 
application. To fix, cancel all subscriptions and asynchronous tasks in the 
componentWillUnmount method.
in DiscoverPage (created by Route)

In my DiscoverPage component:

import React, { Component } from 'react';
import { Switch, Route, Redirect } from 'react-router-dom';
import TvShows from './TvShows';
import Movies from './Movies';
import MovieDetail from "../MoviePage/MovieDetail";
import TvDetail from "../TvShowPage/TvDetail";

class DiscoverPage extends Component {

  constructor(props) {
    super(props);
    this.state = {
      data: {}
    }
  }

  getDataItem = (data) => {
    this.setState({ data: data });
  };

  render() {
    return (
      <Switch>
        <Route exact path="/discover/movie" render={props => <Movies data={this.getDataItem} {...props} />} />
        <Route path="/discover/tv" render={props => <TvShows data={this.getDataItem} {...props} />} />
        <Route path="/movie/:movie" render={props => <MovieDetail data={this.state.data} {...props} />} />
        <Route path="/tv/:tv" render={props => <TvDetail data={this.state.data} {...props} />} />
        <Redirect to="/discover/movie" />
      </Switch>
    );
  }
}

export default DiscoverPage;

In Child component (this case is Movies component):

import React, { Component } from 'react';
import requestApi from '../api';
import MovieList from "../MoviePage/MovieList";

class Movies extends Component {

  constructor(props) {
    super(props);
    this.state = {
      data: {
        page: 1,
        results: []
      }
    }
  }

  componentDidMount() {
    requestApi.fetchData('discover', 'movie').then(response => {
      this.setState({ data: response.data, isLoading: false });
    });
  }

  getMoviebyId = (id) => {
    requestApi.fetchDataById('movie', id).then(response => {
      this.props.data(response.data);
    });
  };

  nextPage = (e) => {
    const page = this.state.data.page + 1;
    requestApi.fetchDataPaginate('discover', 'movie', page).then(response => {
      this.setState({ data: response.data });
    });
  };
  prevPaginate = (e) => {
    const page = this.state.data.page - 1;
    requestApi.fetchDataPaginate('discover', 'movie', page).then(response => {
      this.setState({ data: response.data });
    });
  };

  render() {
    return (
      <div className="container">
        <div className="ss_media">
          <h2 className="title">Discover New Movies & TV Shows</h2>
          <MovieList
            movie={this.getMoviebyId}
            routeProps={this.props}
            prevPaginate={this.prevPaginate}
            nextPaginate={this.nextPage}
            moviesList={this.state.data} />
        </div>
      </div>
    );
  }
}

export default Movies;
Syntle
  • 5,168
  • 3
  • 13
  • 34
Thuan Nguyen
  • 876
  • 3
  • 16
  • 32
  • It looks a little bit strange. `getDataItem` method sets new state. Just to be clear, do you expect `getDataItem` set new state instead of returning `state.data`? – Mikhail Katrin Jun 26 '18 at 09:35
  • hi @MikhailKatrin, getDataItem is a callback function. when Movies component creates a AJAX request then pass data to the parent (this case is DiscoverPage), and then parent component (DiscoverPage) will receive data and set to base state. – Thuan Nguyen Jun 26 '18 at 09:42
  • can you show how do you call `getDataItem` from within any child component? – Karen Grigoryan Jun 26 '18 at 09:42
  • @KarenGrigoryan I was updated more information. Please check above. Thanks – Thuan Nguyen Jun 26 '18 at 09:50

5 Answers5

2

Your DiscoverPage component unmounts when the route changes and after that your invoking getDataItem method. This is why react is throwing error.

I would suggest to use react-router component method instead of render and moving the data to a store which can then be accessed by any component. Refer react-router render methods

Example:

<Route path="/user/:username" component={User}/>
Anu
  • 1,079
  • 8
  • 12
  • Yeah, but how can I pass one of these props to component by this way? – Thuan Nguyen Jun 26 '18 at 09:59
  • 1
    @PhuongThuan You can save the data in the store instead of passing props. – Anu Jun 26 '18 at 10:02
  • @PhuongThuan if you follow the pattern that Anu is suggesting, you wouldn't pass the data through props. Instead, you'd access it in a store. Any given component could read and write this value, so that every component had the same data (which is logically similar to what I think you're trying to do). – cosh Jun 26 '18 at 10:02
  • @Anu how can I using react-router-component instead of render func but still pass props to Child. and somehow child will receive props from parent? – Thuan Nguyen Jun 26 '18 at 10:17
  • @PhuongThuan Are you using some state management library like redux in your app? – Anu Jun 26 '18 at 10:22
  • @Anu, no i only use pure react, not redux or flux or mobx. i write Route like this: `` it should be ok? – Thuan Nguyen Jun 26 '18 at 10:23
1

Some changes need to be done ,the logic you are using is not efficient
ass much as I understood from your question that would be good

import React, { Component } from 'react';
import { Switch, Route, Redirect } from 'react-router-dom';
import TvShows from './TvShows';
import Movies from './Movies';
import MovieDetail from '../MoviePage/MovieDetail';
import TvDetail from '../TvShowPage/TvDetail';

class DiscoverPage extends Component {
    constructor(props) {
        super(props);
        this.state = {
            data: {}
        };
    }
    getALLDATA() {
        //update your state after your request
        request().then(res => {
            this.setState({
                data: res.data
            });
        });
    }
    componentDidMount() {
        this.getALLDATA();
    }
    getDataItem = () => {
        return this.state.data;
    };

    render() {
        return (
            <Switch>
                <Route exact path="/discover/movie" render={props => <Movies data={this.getDataItem} {...props} />} />
                <Route path="/discover/tv" render={props => <TvShows data={this.getDataItem} {...props} />} />
                <Route path="/movie/:movie" render={props => <MovieDetail data={this.state.data} {...props} />} />
                <Route path="/tv/:tv" render={props => <TvDetail data={this.state.data} {...props} />} />
                <Redirect to="/discover/movie" />
            </Switch>
        );
    }
}

export default DiscoverPage;
jadlmir
  • 465
  • 1
  • 6
  • 16
  • my getDataItem function accepts one parameter. I was called AJAX request in Movies component then pass the result to parent Component (this case is DiscoverPage ) – Thuan Nguyen Jun 26 '18 at 10:04
  • logically you should do the inverse , you should get the data in parent and pass them to the children anw if that what you wish try – jadlmir Jun 26 '18 at 10:06
1

The warning you're getting is telling you that you can't call setState on an unmounted component, i.e. a component that's not currently rendered. Most likely, one of your other components (Movies, TvShows, etc.) is calling the method you pass as the data prop. When one of these components is rendered, the DiscoveryPage component is not, so calling the getDataItem through the data prop will result in DiscoveryPage's setState method to be called, when it's not rendered. To fix this (assuming that what you want to do is pass data from component to component) I'd suggest maybe using localStorage to store a shared variable (or, as @Anu suggested, some other kind of store).

I don't understand what you mean about "in arguments", but, answering your question, you can call setState in any component... method (even though calling it in componentWillUnmount makes little to no sense, since this methos is only called when the component is about to be unmounted).

EDIT: Look at the Movie component. Using localStorage as our store, in the getMovieById method, instead of:

getMoviebyId = (id) => {
    requestApi.fetchDataById('movie', id).then(response => {
        //this next sentence will call DiscoveryPage's setState
        //DiscoveryPage is not mounted, so a warning will be raised
        this.props.data(response.data);
    });
};

you could have something like:

getMoviebyId = (id) => {
    requestApi.fetchDataById('movie', id).then(response => {
        //any component can access this store
        localStorage.setItem("data", response.data);
    });
};

and then, in some other component, or method, access it like so:

localStorage.getItem("data");

Your code is a little bit confusing. I'm having a little trouble understanding where you're using the DiscoveryPage's data, the one you're setting when you call the data method. But anyway, hope this helps.

Just one more side note: When using localStorage, it's good practice to manage it, by which I mean calling localStorage.removeItem("data") when you're done using the data value. This is best done, for example, in some componentWillUnmount method.

cosh
  • 470
  • 1
  • 4
  • 15
  • Thank you!. Its work, but more questions are where should I put `localStorage` in my component. I put it in `componentDidMount` and it works very well. I also unsubscribed it by add this hook. `componentWillUnmount() { localStorage.removeItem("data"); }` it fine? – Thuan Nguyen Jun 26 '18 at 10:58
  • Glad to hear! :) yes, that was the approach I was suggesting. Just take care not to remove it while you still need it in another component. Try figuring out a way of having a parent component for the components that need it, and managing it there. Also bear in mind that this is, IHMO, an error-prone pattern, so I also recommend considering other ways of managing stores, e.g. react-redux. – cosh Jun 26 '18 at 11:15
0

I think you are missing to import BrowserRouter from react-router-dom

import { Switch, Route, Redirect, BrowserRouter } from 'react-router-dom'; // Add BrowserRouter

class DiscoverPage extends Component {

...
render() {
    return (
    <BrowserRouter> // Add this 
        <Switch>
            <Route exact path="/discover/movie" render={props => <Movies data={this.getDataItem} {...props} />} />
            <Route path="/discover/tv" render={props => <TvShows data={this.getDataItem} {...props} />} />
            <Route path="/movie/:movie" render={props => <MovieDetail data={this.state.data} {...props} />} />
            <Route path="/tv/:tv" render={props => <TvDetail data={this.state.data} {...props} />} />
            <Redirect to="/discover/movie" />
        </Switch>
    </BrowserRouter>
    );
}
}
Damian Peralta
  • 1,846
  • 7
  • 11
0

We can set state in componentDidMount like:

componentDidMount(){
    var {state, setState} = this.props;
    setState({doNotRender: true});
}

But instead use componentDidUpdate like:

componentDidUpdate(){
    var {state, setState} = this.props;
    if(!state.doNotRender){ //any_condition ur choice
       setState({doNotRender: true});
    }
}
sametcodes
  • 583
  • 5
  • 8