Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[docs] Loading Data Asynchronously #8883

Conversation

dashtinejad
Copy link
Contributor

@gaearon Related to this pull request #8098, I've rewritten the whole process with axios. I've also implemented the cancellation.

Copy link
Contributor

@lacker lacker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like there are still some race conditions in these code examples. I think rather than offering a few different code examples, none of which is both complete + bug-free, it would be better to just show the simplest code that is bug-free, even if there are some slightly awkward cancellation and error-handling bits, and then explain what's happening in the text.


componentDidMount() {
axios.get('https://api.github.com/users/facebook/repos')
.then(response => this.setState({ repos: response.data }))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens here if the component has unmounted before this request completes?

Copy link

@stereobooster stereobooster Jun 27, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose we need to add cancel to componentWillUnmount.

componentDidMount() {
   let CancelToken = axios.CancelToken;
   let cancel
   let cancelToken = CancelToken(c => cancel = c)
   
   this.setState({ cancel: cancel })

   axios.get('https://api.github.com/users/facebook/repos', { cancelToken })
     .then(response => this.setState({ repos: response.data }))
     .catch(error => console.log(error));
 }

componentWillUnmount() {
  this.state.cancel()
}

UPD there is example with cancelation lower


Often, the data that a component needs is not available at initial render. We can load data asynchronously in the [`componentDidMount` lifecycle hook](/react/docs/react-component.html#componentdidmount).

In the following example we use the [axios](https://github.com/mzabriskie/axios) to retrieve information about Facebook's Repos on GitHub and store them in the state. So first install it:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you say "repositories" instead of "Repos"? I think a lot of people will not instinctively know what a "repo" is.

this.state = {repos: []};
}

fetchRepos() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this fetchRepos function supposed to have a particular behavior in terms of doing things in the right order? Does axios guarantee that?

}

componentDidUpdate(prevProps) {
if (this.props.username != prevProps.username) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't this lead to, if you first load repos for foo, and then for bar, on the immediate load of bar it will show the foo repos under a header that says "bar repo"?

fetchRepos() {
// cancel the previous request
if (typeof this._source != typeof undefined) {
this._source.cancel('Operation canceled due to new request.')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible that this._source has already completed by the time this line of code runs? That seems possible to me.

@lacker
Copy link
Contributor

lacker commented Feb 1, 2017

The frequency of race conditions while people try to implement a basic network-request example makes me wonder if actually, doing network requests in componentWillMount is a bad idea. We don't really do that at Facebook frequently, do we? (I could be wrong there.) I feel like my personal recommendation at least to people would be, avoid doing this if you want your code to be correct; prefer using mechanisms like redux that make it easier to avoid race conditions in your data processing.

class Repos extends React.Component {
constructor(props) {
super(props);
this.state = {repos: []};

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about loading state e.g. spinner?

if (axios.isCancel(error)) {
console.log('Request canceled', error);
} else {
console.log(error);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

state error - show error message. Not that much code, but will encourage programmers to implement error messages and think of all states of component with async load

@stereobooster
Copy link

stereobooster commented Jun 27, 2017

I think rather than offering a few different code examples, none of which is both complete + bug-free, it would be better to just show the simplest code

Yes please.

Also worth to mention in documentation, that there is ongoing discussion in ES committee about CanceTokens and if it will make it to spec, it is expected that Fetch API will support it too. Axios is nice, but native API (with polyfill if required) would be nicer.

Related: https://facebook.github.io/react/blog/2015/12/16/ismounted-antipattern.html

UPD Combined example. That is so much boilerplate, I feel like it worth to wrap it in a HOC

import axios from 'axios';

class Repos extends React.Component {
  constructor(props) {
    super(props);
    this.state = {
      repos: [],
      networkState: 'loading'
    };
  }

  fetchRepos() {
    // cancel the previous request
    if (this.source) {
      this.source.cancel('Operation canceled due to new request.')
    }

    // save the new request for cancellation
    this.source = axios.CancelToken.source();

    this.setState({networkState: 'loading' })

    let url = `https://api.github.com/users/${this.props.username}/repos`
    axios.get(url, { cancelToken: this.source.token })
      .then(response => this.setState({ repos: response.data, networkState: 'loaded' } ))
      .catch(error => {
        if (axios.isCancel(error)) {
          this.setState({ networkState: 'canceled' })
        } else {
          this.setState({ networkState: 'errored' })
        }
     });
  }

  componentDidMount() {
    this.fetchRepos();
  }

  componentDidUpdate(prevProps) {
    if (this.props.username != prevProps.username) {
      this.fetchRepos();
    }
  }

  componentWillUnmount() {
    if (this.source) {
      this.source.cancel('Operation canceled due to unmount.')
    }
  }

  render() {
    return (
      <div>
        <h1>Repos by {this.props.username}</h1>
        { 
          this.state.networkState === 'loaded' ?
            this.state.repos.map(repo => <div key={repo.id}>{repo.name}</div>) :
            this.state.networkState
        }
      </div>
    );
  }
}

class App extends React.Component {
  constructor(props) {
    super(props);
    this.state = {username: 'facebook'};
  }

  render() {
    return (
      <div>
        <button onClick={() => this.setState({username: 'facebook'})}>Facebook</button>
        <button onClick={() => this.setState({username: 'microsoft'})}>Microsoft</button>
        <button onClick={() => this.setState({username: 'google'})}>Google</button>
        <Repos username={this.state.username} />
      </div>
    );
  }
}

ReactDOM.render(<App />, document.getElementById('app'))

@stereobooster
Copy link

Small update: fetch now supports abort https://developers.google.com/web/updates/2017/09/abortable-fetch

Need to make sure that polyfill (especially that is bundles with c-r-a) supports it.

@bvaughn
Copy link
Contributor

bvaughn commented Oct 6, 2017

Thank you for submitting this PR! 😄

Unfortunately the documentation and source code for reactjs.org has been moved to a different repository: https://github.com/reactjs/reactjs.org

If you are willing, please open a new PR there. Sorry for the inconvenience!

(For more backstory on why we moved to a new repo, see issue #11075)

@bvaughn bvaughn closed this Oct 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants