Add a guide on loading data asynchronously #8098

rthor · 2016-10-25T23:45:06Z

This guide demonstrates how to load data asynchronously in a React component. I wasn't quite sure where a link to the guide should be placed. Is the Advanced Guides section a good fit?

It addresses the AJAX part in #8060.

rickbeerendonk · 2016-10-26T14:12:29Z

I like the example and the usage.

In #8060 it also says "mention cancellation issues". Important, because you don't know when fetch returns:

It might be that an old fetch is taking longer and will overwrite the results of a fetch started more recently.
Component is unmounted before the fetch finished.

rthor · 2016-10-26T14:52:11Z

Yeah good point, I forgot to mention that.

Wondering if a Caveat section would suffice since this is not really a problem with React, but rather async execution in general? Fetch, f.ex. has no built-in way of canceling [the promise] unless you wrap it somehow, so I wonder if a code example would be to complex.

Perhaps, fetch is just not the way to go if we want to give an example on cancellation. Other methods might be better at demonstrating how to go about it. Something with cancellation built-in.

rthor · 2016-10-26T14:54:58Z

But if mentioning the issues is enough, then I think a Caveat section is the way to go.

rickbeerendonk · 2016-10-26T15:00:36Z

Caveat section would be fine with me, don't know what @gaearon thinks about this though.

A standard for canceling fetch is still in process. I suspect your documentation will double in size if you would give an example of a workaround. That would be too much of a distraction.

rthor · 2016-10-26T22:53:03Z

Sending an update tomorrow with the cancellation issues.

gaearon · 2016-10-27T12:17:06Z

This looks like a really great start.

gaearon

My comments so far. We'll definitely need to explain the issue with cancellation.

gaearon · 2016-10-27T12:17:34Z

docs/docs/loading-data-asynchronously.md

+
+Often, the data that a component needs is not available at initial render. We can load data asynchronously in the `componentDidMount` [lifecycle hook](https://facebook.github.io/react/docs/react-component.html#componentdidmount).
+
+In the following example we use the `fetch` API to retrieve information about Facebook's Gists on GitHub and store them in the state.


fetch should link to MDN.

We should mention that this is a browser API but it requires a polyfill. Link to the polyfill. Mention that if you use Create React App, polyfill is already included.

gaearon · 2016-10-27T12:17:52Z

docs/docs/loading-data-asynchronously.md

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


Please leave the domain out of the links. Keep it as /react/docs/react-component.html#componentdidmount.

gaearon · 2016-10-27T12:18:22Z

docs/docs/loading-data-asynchronously.md

+```javascript{10-14}
+class Gists extends React.Component {
+  constructor(props) {
+    super(props);


Nit: let's write it shorter,

constructor(props) { super(props); this.state = {gists: []}; }

gaearon · 2016-10-27T12:18:38Z

docs/docs/loading-data-asynchronously.md

+
+  render() {
+    const { gists } = this.state;
+


Nit: let's not put a newline here.

gaearon · 2016-10-27T12:19:18Z

docs/docs/loading-data-asynchronously.md

+    return (
+      <div>
+        <h1>Gists by facebook</h1>
+        {gists.map(gist => <p><a href={gist.html_url}>{gist.id}</a></p>)}


Nit: let's put the JSX on next line.

{gists.map(gist => <p>...</p> )}

gaearon · 2016-10-27T12:21:32Z

docs/docs/loading-data-asynchronously.md

+    );
+  }
+
+  /* ... */


What's here? I'd rather avoid ambiguous "old stuff" blocks.

Just the constructor. Should I repeat it instead? Wasn't sure what your preference would be.

gaearon · 2016-10-27T12:23:21Z

docs/docs/loading-data-asynchronously.md

+prev: typechecking-with-proptypes.html
+---
+
+


We need to explain the key concept somewhere early. We need to hammer home the idea that in React, any update means a change in the state. It doesn't have any special capabilities for handling AJAX: if you want to change the UI as a result of data arriving, you need to change the state.

gaearon · 2016-10-27T12:23:46Z

docs/docs/loading-data-asynchronously.md

+  /* ... */
+}
+```
+


We should have the last section that shows the same with async/await I think. Async/await is enabled in Create React App by default.

In that case, shouldn't all the examples feature async / await? Or is it better to demonstrate both methods?

gaearon · 2016-10-27T12:24:19Z

docs/_data/nav_docs.yml

@@ -32,6 +32,8 @@
      title: JSX In Depth
    - id: typechecking-with-proptypes
      title: Typechecking With PropTypes
+    - id: loading-data-asynchronously
+      title: Loading Data Asynchronously
    - id: refs-and-the-dom


If we put it here, "refs and DOM" will need its prev link changed.

Yeah, I realized that after I made the PR. Didn't want to change it in case this guide should be elsewhere. Should we keep it here then?

Going back to it, I saw that other guides in the same section don't have previous links. I removed the one in this article to keep consistency.

rthor · 2016-10-27T23:04:52Z

Thanks for the feedback! I've gone through everything that you pointed out except for the guide's intro. Working on that ATM.

rthor · 2016-10-27T23:09:26Z

Also, I'll be happy to add async / await. Only concern is that it'll confuse the reader unless all the examples and CodePen are written that way. What are your thoughts on the matter, @gaearon ?

gaearon · 2016-10-29T12:13:53Z

Only concern is that it'll confuse the reader unless all the examples and CodePen are written that way.

I think it'll be fine as a last section with a clear disclaimer we're using something that isn't part of ES2015 yet and needs Babel transform unless you're targeting very modern browsers.

gaearon · 2016-10-29T12:18:37Z

docs/docs/loading-data-asynchronously.md

+
+## Pitfalls
+
+An old promise can be pending when a newer promise fulfills. This can cause the old promise to override the results of the new one. If a promise is pending when a component is updated, the pending promise should be cancelled before a new one is created.


Technically promises can't be "cancelled" so this might read confusing. Maybe "the result of the first promise should be ignored".

gaearon · 2016-10-29T12:26:15Z

docs/docs/loading-data-asynchronously.md

+
+An old promise can be pending when a newer promise fulfills. This can cause the old promise to override the results of the new one. If a promise is pending when a component is updated, the pending promise should be cancelled before a new one is created.
+
+Additionally, a component can unmount while a promise is pending. To avoid unexpected behavior and memory leaks when this happens, be sure to also cancel all pending promises in the `componentWillUnmount` [lifecycle hook](/react/docs/react-component.html#componentwillunmount).


Same here, you can't cancel promises. Also it isn't strictly causing memory leaks, just unnecessary requests.
Maybe:

Additionally, a component can unmount while a promise is pending. React warns you if you call setState() on unmounted components to prevent memory leaks. Some data fetching APIs allow you to cancel requests, and this is preferable when component unmounts. For APIs such as fetch() that don't offer a cancellation mechanism, you would need to keep track of whether component is mounted to avoid seeing warnings. Here is how we could implement this:

Then show how to set this.isUnmounted = true; in componentWillUnmount() and change fetch() callback to exit if this flag is set or if current username in props doesn't match the one we were fetching. This would solve both caveats.

As a final example you could then show a more concise way of doing the same with a library that supports cancellation like axios.

rthor · 2016-10-30T22:54:57Z

Sorry for how long it took to address those things. I don't have time to add an example on cancellation using a library right now but I'ld be happy to do it later. If someone else wants to do it, then that's alright too. Otherwise, I think that this is ready.

gaearon · 2016-11-08T20:18:56Z

I don't have enough time to finish reviewing this, leaving it to @lacker.
Thank you so much!

3rd-party => third party

put name in link

lacker · 2016-11-09T23:26:12Z

docs/docs/loading-data-asynchronously.md

+  }
+
+  componentDidMount() {
+    fetch('https://api.github.com/users/facebook/gists')


I think this code is not using fetch appropriately - you should be checking response.ok to see if there is an error. As is, if there is a 4xx or 5xx error I think it will fail in some weird way during parsing. Even just rejecting a promise would be OK here IMO but fetch actually does not reject the promise on server failure.

Yeah sorry, my bad.

lacker · 2016-11-09T23:28:07Z

docs/docs/loading-data-asynchronously.md

+We can simplify the `fetchGists` method by using the [`async / await`](https://tc39.github.io/ecmascript-asyncawait/) feature:
+
+```javascript{1,3-4}
+async fetchGists() {


I think the best practice is actually to write some helper that uses fetch. Then the nature of this async/await example is different. I think it's still handy to have an example with async/await here though.

Helper to handle errors and bad requests then? eg

function safeFetch(url, options) { return fetch(url, options).then(response => { if (!response.ok) { throw new Error(response.statusText) } return response.json() }) }

So the async code would be:

async fetchGists() { const { username } = this.props; try { const gists = await safeFetch(`https://api.github.com/users/${username}/gists`); this.setState({gists}); } catch (error) { // Request failed... } }

lacker · 2016-11-09T23:30:25Z

docs/docs/loading-data-asynchronously.md

+> 
+> `async / await` is still a proposal for the ECMAScript spec and therefore hasn't been implemented in most browsers. To use it today, a [Babel](http://babeljs.io/docs/plugins/transform-async-to-generator/) (or similar) transform is needed. If you're using Create React App, it works by default.
+
+## Pitfalls


So basically you could not use any of the previous examples in production because they do not handle the cases where promises are fulfilled out of order, or the component unmounts. Is it even a good idea to explain those paradigms? It does not seem like a great idea to provide sample code that is actually buggy.

Is there a simple way to write code that loads data asynchronously that actually does not have any of these race condition bugs?

Not that I'm aware of, no. Would you prefer that we bake the pitfalls section into each example? Ie provide "bug-free" code from the get-go even if it might be a bit verbose and will probably be explaining to much in one pass for a newcomer.

lacker · 2016-11-09T23:31:34Z

I tweaked some minor grammatical issues and also provided feedback. Overall I am concerned that if we only provide examples that contain race condition bugs, we are not actually documenting "best practices". We need to promote code that does not have parallelism bugs in it.

hedgerh

Great work so far. I have some ideas for ways to improve the copy. Would you mind if I took a stab at it?

hedgerh · 2016-11-16T10:21:11Z

docs/docs/loading-data-asynchronously.md

+}
+```
+
+We can extract the common code in `componentDidMount` and `componentDidUpdate` into a new method, `fetchGists`, and call that in both lifecycle hooks.


I think we should get rid of this refactor step. It adds 40 lines just to explain to the reader that if they write the same code twice, they should probably turn it into a function. The other option is for the first example to already have the fetchGists method.

While I agree that this adds a lot to the guide, I think that this way is more in tone with the rest of the documentation. Ie show a "simplified" version first and gradually refactor towards best practices.

hedgerh · 2016-11-16T10:24:06Z

docs/docs/loading-data-asynchronously.md

+
+[Try it out on CodePen.](http://codepen.io/rthor/pen/kkqrQx?editors=0010)
+
+We can simplify the `fetchGists` method by using the [`async / await`](https://tc39.github.io/ecmascript-asyncawait/) feature:


We should get rid of all this stuff about async/await, too. It's really out of scope. Our readers are busy learning React, and they either already know about async/await, or shouldn't worry about learning about async/await right here.

Good point. But Dan wanted this in the guide.

Async/await is enabled in Create React App by default.

@gaearon: would you mind elaborating on why we should include a section on async/await?

It seems like something better suited for async/await documentation, since it's a pretty trivial example.

It seems like something better suited for async/await documentation, since it's a pretty trivial example.

Not sure what you mean by async/await documentation. Is this something we would write? Which example is trivial?

I think that if we don't show async/await "works" inside components then many people won't realize that. I've seen many surprised reactions to async componentDidMount() because people think it's something React has to "support" as opposed to just being a language feature. So I think it's nice to provide at least a short section giving you enough clues to get started with using it.

hedgerh · 2016-11-16T10:31:07Z

docs/docs/loading-data-asynchronously.md

+  }
+
+  componentDidUpdate(prevProps) {
+    // Make sure that the `username` prop did change before


We probably only need these comments in the example for the componentDidUpdate example, but no biggie either way.

hedgerh · 2016-11-16T10:42:59Z

docs/docs/loading-data-asynchronously.md

+
+An old promise can be pending when a newer promise fulfills. This can cause the old promise to override the result of the new one. If a promise is pending when a component is updated, the result of the first promise should be ignored before a new one is created.
+
+Additionally, a component can unmount while a promise is pending. React warns you if you call `setState` on unmounted components to prevent memory leaks. Some data fetching APIs allow you to cancel requests, and this is preferable when a component unmounts. For APIs such as `fetch` that don't offer a cancellation mechanism, you need to keep track of whether the component is mounted to avoid seeing warnings. Here is how we could implement this:


We should first mention that the best solution to this issue is to resolve whatever is causing this unmounting issue. API calls should be happening as high up in your component tree as possible. If your top-level component is so far up the tree that you can't make the API call there and pass down the data, that's when I'd start considering keeping my state outside of React.

So what's your suggested solution to callback race conditions - just don't unmount a component that's loading data? I don't like having a "pitfalls" section that just states that something is a problem for everyone without showing how to solve it - we should just not have a "pitfalls" part and instead explain "how to do things right".

I'm not sure what you mean by resolving the unmounting issue as it can be a completely correct behaviour and has nothing to do with the request itself.

API calls should be happening as high up in your component tree as possible.

I would argue against this approach. If data is passed down the entire tree, without defining componentShouldUpdate at every step, the entire app will be rerendering with every response.

I was imagining some scenario that could be fixed by restructuring your code. For instance, if it'd be possible to move the API call up to the immediate parent, and pass the data down to the component instead. Was I way off base there? :P

Cancellation is obviously the solution when it's unavoidable.

Also, out of curiosity, what are some common use cases where this occurs for people? I understand how it could happen, but I never really encounter it. Route changes was one that came to mind.

Route changes is the easiest case, yes. Go forward, then go back, oops, the wrong thing has arrived.

rthor · 2016-11-16T20:50:46Z

Thanks @lacker for reviewing! I'm quite busy with work at the moment and don't have time to address these issues right now.

rthor · 2016-11-20T12:53:33Z

I think these points are all good, especially the race condition ones. And, I think you're right in that showing buggy code is not optimal. Perhaps I'm going about this the wrong way. The intent of the guide is to address ajax and React but it's turning into a wormhole of JS specific workarounds, not React.

The intro section could be expanded to iterate on all the sections of this guide:

Use lifecycle hooks for fetching data.
Rerender the app based on fetched data by setting the state.
An explanation of how to deal with cancelation.

This all followed by a complete code demo that is not buggy and follows best practices. Short explanations of certain snippets, or code blocks, could be either inlined via comments or explained in more detail at the end of the document.

This demo could then be showcased using the async/await syntax and/or perhaps a third party lib for cancelation.

brigand · 2016-11-20T14:37:38Z

Just throwing an idea out there, not sure if this is good. It ensures the rest of the chain is just dropped if the component unmounts.

await safeFetch(url)
  .then(...continueIfMounted(this))
  .then((body) => this.setState({body})

// or
const body = await safeFetch(url).then(...continueIfMounted(this))

function continueIfMounted(instance) {
  const originalUnmount = instance.componentWillUnmount
  let isMounted = true
  instance.componentWillUnmount = (...args) => {
    isMounted = false
    if (originalUnmount) {
      return originalUnmount.apply(instance, args)
    }
  }

  return [
    (x) => isMounted ? x : new Promise(() => {}),
    (x) => isMounted ? Promise.reject(x) : new Promise(() => {}),
  ]
}

lacker · 2016-11-29T19:24:08Z

I think the real problem here is that using React + fetch is actually pretty painful. I didn't realize it before all this discussion happened, but there really is no clean way to avoid the problems that come from race conditions with multiple outstanding fetches, and fetches that return after the component unmounts. So I would no longer recommend that people use React + fetch. The fundamental problem is that you cannot cancel a request - if you can, then the logic is simple - cancel any previous request when you send a new request, or the component unmounts. So I think it would be better for these docs to just use a library that supports cancellation, like axios, and point out these two gotchas so that if people do want to write data loading with fetch they are aware of the gotchas.

gaearon · 2016-11-29T19:51:29Z

Races from multiple fetches while component is mounted are solved the same way unmounting is solved: by cancellation.

hedgerh · 2016-12-06T10:59:41Z

Ahh okay. That makes sense. I just wasn't sure how it was a react concern.

…

On Tue, Dec 6, 2016 at 5:46 AM Dan Abramov ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In docs/docs/loading-data-asynchronously.md <#8098>: > + const { gists } = this.state; + return ( + <div> + <h1>Gists by {username}.</h1> + {gists.map(gist => + <p><a href={gist.html_url}>{gist.id}</a></p> + )} + </div> + ); + } +} +``` + +[Try it out on CodePen.](http://codepen.io/rthor/pen/kkqrQx?editors=0010) + +We can simplify the `fetchGists` method by using the [`async / await`](https://tc39.github.io/ecmascript-asyncawait/) feature: It seems like something better suited for async/await documentation, since it's a pretty trivial example. Not sure what you mean by async/await documentation. Is this something we would write? Which example is trivial? I think that if we don't show async/await "works" inside components then many people won't realize that. I've seen many surprised reactions to async componentDidMount() because people think it's something React has to "support" as opposed to just being a language feature. So I think it's nice to provide at least a short section giving you enough clues to get started with using it. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#8098>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACaEeFX62vqjt991w0Br-AXeRu83uKhSks5rFTz4gaJpZM4Kgm4E> .

gaearon · 2017-01-04T16:29:22Z

@rthor @dashtinejad Would either of you be interested working on this but with axios? I'm sorry for the churn here but given the sad status of cancellation in fetch I think we should just make a good axios guide with cancellation and no race conditions.

dashtinejad · 2017-01-08T08:28:41Z

@gaearon Yes, of course, why not. I'll work on it.

lacker · 2017-02-01T21:03:24Z

Let's close this one and continue the discussion on #8883

Add a guide on loading data asynchronously

b06e0d5

facebook-github-bot added GH Review: review-needed CLA Signed labels Oct 25, 2016

Add missing word

382fee4

gaearon self-assigned this Oct 27, 2016

gaearon reviewed Oct 27, 2016

View reviewed changes

rthor added 5 commits October 27, 2016 17:55

Relative links

b545300

More concise code snippets

34aedac

Pitfalls section that explains cancellation issues

4c92f0e

Add information on the fetch API

55ead66

Other advanced doc files don't have previous links

4b1fa64

gaearon reviewed Oct 29, 2016

View reviewed changes

rthor added 3 commits October 30, 2016 21:42

Example that illustrates how to ignore a response

461082c

Add an async / await example

763c790

Add an intro

c1a36a1

rthor mentioned this pull request Nov 1, 2016

[New Docs] Wanted Guides #8060

Closed

13 tasks

gaearon assigned lacker and unassigned gaearon Nov 8, 2016

little style tweak

72632e9

3rd-party => third party

tweak

a9a6007

put name in link

lacker reviewed Nov 9, 2016

View reviewed changes

Update loading-data-asynchronously.md

74cc978

lacker reviewed Nov 9, 2016

View reviewed changes

lacker added GH Review: needs-revision and removed GH Review: review-needed labels Nov 9, 2016

gaearon mentioned this pull request Nov 13, 2016

[docs] Ajax Documentation #8276

Closed

hedgerh suggested changes Nov 16, 2016

View reviewed changes

zpao added the Component: Documentation & Website label Nov 17, 2016

dashtinejad mentioned this pull request Jan 30, 2017

[docs] Loading Data Asynchronously #8883

Closed

lacker closed this Feb 1, 2017

gaearon mentioned this pull request Feb 11, 2017

Add docs for using Promises and making AJAX requests facebook/create-react-app#531

Closed


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

		In the following example we use the `fetch` API to retrieve information about Facebook's Gists on GitHub and store them in the state.


		## Pitfalls

		An old promise can be pending when a newer promise fulfills. This can cause the old promise to override the results of the new one. If a promise is pending when a component is updated, the pending promise should be cancelled before a new one is created.


		An old promise can be pending when a newer promise fulfills. This can cause the old promise to override the results of the new one. If a promise is pending when a component is updated, the pending promise should be cancelled before a new one is created.

		Additionally, a component can unmount while a promise is pending. To avoid unexpected behavior and memory leaks when this happens, be sure to also cancel all pending promises in the `componentWillUnmount` [lifecycle hook](/react/docs/react-component.html#componentwillunmount).


		[Try it out on CodePen.](http://codepen.io/rthor/pen/kkqrQx?editors=0010)

		We can simplify the `fetchGists` method by using the [`async / await`](https://tc39.github.io/ecmascript-asyncawait/) feature:

		prev: typechecking-with-proptypes.html
		---

Add a guide on loading data asynchronously #8098

Add a guide on loading data asynchronously #8098

Conversation

rthor commented Oct 25, 2016

rickbeerendonk commented Oct 26, 2016

rthor commented Oct 26, 2016

rthor commented Oct 26, 2016 • edited Loading

rickbeerendonk commented Oct 26, 2016

rthor commented Oct 26, 2016

gaearon commented Oct 27, 2016

gaearon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rthor Oct 27, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rthor commented Oct 27, 2016

rthor commented Oct 27, 2016

gaearon commented Oct 29, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rthor commented Oct 30, 2016

gaearon commented Nov 8, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rthor Nov 19, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rthor Nov 19, 2016 • edited Loading

Choose a reason for hiding this comment

lacker commented Nov 9, 2016

hedgerh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rthor commented Nov 16, 2016

rthor commented Nov 20, 2016

brigand commented Nov 20, 2016 • edited Loading

lacker commented Nov 29, 2016

gaearon commented Nov 29, 2016

hedgerh commented Dec 6, 2016 via email

gaearon commented Jan 4, 2017

dashtinejad commented Jan 8, 2017

lacker commented Feb 1, 2017

rthor commented Oct 26, 2016 •

edited

Loading

rthor Oct 27, 2016 •

edited

Loading

rthor Nov 19, 2016 •

edited

Loading

rthor Nov 19, 2016 •

edited

Loading

brigand commented Nov 20, 2016 •

edited

Loading