3

I am fetching data from my API and need to construct a list based on the received data. The problem is, the rendering happens earlier than the fetching in the constructor is completed so the this.todo sent to List component happens to be empty. What is the correct way to handle this?

import React, { Component } from 'react';
import {render} from 'react-dom';
import Header from './header.jsx'
import List from './list.jsx'

class App extends Component {
  constructor(props) {
    super(props)
    this.todos = []

    const url = "http://localhost:3000/api/todos/"
    fetch(url)
    .then(res => res.json())
    .then((data) => {
      this.todos = data;
    })
    .catch(console.error);
  }

  render () {
    return (
      <div>
        <Header />
        <List tasks={this.todos}/>
      </div>
    )
  }
}

render(<App/>, document.getElementById('app'));
nem035
  • 34,790
  • 6
  • 87
  • 99
Eduard Avetisyan
  • 471
  • 1
  • 4
  • 20
  • It's generally just considered bad practice to put asynchronous operations inside a constructor. This is because there's just not a clean way to communicate back a promise from which the caller can ascertain when the async operation is done or if it had an error. See [Node constructor not blocking](https://stackoverflow.com/questions/45445808/node-code-not-blocking/45451111#45451111) for three separate design solutions. My favorite is to use a factory function that creates the object, then calls a method on it to do the async stuff and returns a promise who's resolve value is the new object. – jfriend00 Jan 21 '18 at 00:23

1 Answers1

1

You usually want to give some signal to the user that the data is loading. You also want to make sure to differentiate between empty data and data not received.

You also want this data to be part of the component state and not on the class instance itself, to allow the component to re-render when the data is received.

=> this.state.todos instead of this.todos

class Example extends React.Component {
  constructor(props) {
    super(props)
    this.state = {
      todos: null
    }
    setTimeout(() => {
      this.setState({
        todos: [1,2,3]
      })
    }, 1000);
  }

  render () {
    return (
      <div>
        {this.state.todos
          ? <div>{this.state.todos.toString()}</div>
          : <div>Loading todos...</div>
        }
      </div>
    )
  }
}

ReactDOM.render(<Example/>, document.getElementById('app'));
<script src="https://cdnjs.cloudflare.com/ajax/libs/react/15.1.0/react.min.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/react/15.1.0/react-dom.min.js"></script>
<div id="app"></div>

There are many other improvements to be made here. You can potentially cache data and show it later on without needing to fetch it from the network, then you can grab any new data behind the scenes. If a spinner doesn't work, you can perhaps use a placeholder of sorts. It would all depend on the type of data, the persistence of it, how much the user interacts with it, how much does the data change, etc.

nem035
  • 34,790
  • 6
  • 87
  • 99
  • react documents says never use setState in constructor https://reactjs.org/docs/react-component.html#setstate Could you please refine answer more? – Shankar Gurav Jan 17 '20 at 05:34