React: Using setState inside a "function as child component" generates a warning about functions not being valid children

Created on 5 Oct 2017  Ā·  12Comments  Ā·  Source: facebook/react

Do you want to request a feature or report a bug?

Reporting a bug.

What is the current behavior?

_Context: React Native app, but the error isn't related to the renderer_

I'm using a handcrafted <Fetch path="/api/route" render={this.renderChildren} /> component which fetches data and passes it to a ''function as child component", which is rendered. However when using setState to save the call result, I get a pretty interesting error as you can see:

screenshot of the code leading to the error

šŸ‘‡ gives you šŸ‘‡

screenshot of the error

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem via https://jsfiddle.net or similar (template for React 16: https://jsfiddle.net/Luktwrdm/, template for React 15: https://jsfiddle.net/hmbg7e9w/).

I tried to remove all the business logic and reproduce the error from my React Native app. Here is a CodeSandbox which fully reproduce it: https://codesandbox.io/s/pmo6okkz1x.

What is the expected behavior?

I expected not to have this error.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?

From what I know: this error occurs on any React renderer (got it on React Native in the first place), and started the project with React 15.5.

Related issues
Might be related to: #11081, #11086.


Ping: @gaearon

Most helpful comment

Happy to help!

All 12 comments

I’m not sure I understand what you’re trying to do, but it seems like youā€˜re calling setState inside render, which is a bad idea for obvious reasons…

This error makes it pretty clear:
screen shot 2017-10-05 at 18 06 46

Even smaller reproduction: https://codesandbox.io/s/wq69qyvrx8

Yes, the problem is that _saveData in your example is called during rendering. So you start rendering, call setState, that starts rendering again, you call setState again, etc. This results in an infinite loop.

If your Fetch component takes care of keeping the state, and then calling its render prop to render that data, there's no need for your App component to try to do the same thing. Just render what Fetch gives you:

import React from 'react'
import { render } from 'react-dom'
import Fetch from './Fetch'

class App extends React.Component {
  _renderChildren = (data) =>
    <span>{JSON.stringify(data)}</span>

  render() {
    return (
      <Fetch
        path="https://unsplash.it/list"
        render={this._renderChildren}
      />
    );
  }
}

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

Here is a working sandbox: https://codesandbox.io/s/48rrpxq33x

Thanks @Tom-Bonnike @gaearon for quick answers. That's actually a little bit different from what I was trying to achieve there as you saw. The idea was to fetch data with <Fetch />, save it and then use that data with any other component that would need it. But your answers make sense with what I've put there indeed.

The idea was to fetch data with , save it and then use that data with any other component that would need it.

The solution in your case would be to modify the render callback to pass that data down to any components that need it. I know it’s a bit counter-intuitive but since you went with this pattern, there’s no need to bring data ā€œupā€ now in your component. Just tell Fetch what to render.

Thanks for your advice. That's exactly what I'm doing now:

<Fetch path="/api/route">
  {(data) => <MyComponent content={data} />}
</Fetch>

I'm just exploring different patterns to see if there is another approach that could be more interesting. Anyhow thanks again for your time!

One way to think about it is that for every piece of data there should be only one component that holds it in the state.

So you need to pick: either it's Fetch (and you can only use what it gives you via the render prop), or it's App (and in this case you need a different Fetch API that would call handleChange when the data loads instead of trying to keep it in its own state).

I'm leaning toward the second option, just to see what I can achieve with it.

In the first place, I wanted to lift my state up (using Fetch) and share that data. But that's probably where my mistake was, because it would lead to a ā€˜strangeā€˜ pattern: a _sibling_ (Fetch) fetching data, lifting it up inside the _parent's state_ so that its siblings could exploit that data... After a second thought, this doesn't seem ā€˜naturalā€˜.

On another note: do we also have access to handleChange/onChange inside any React Native component or is it tied to the DOM? I was thinking to onLayout on the <View /> component, but maybe that's not exactly what I'd want.

But that's probably where my mistake was, because it would lead to a ā€˜strangeā€˜ pattern: a sibling (Fetch) fetching data, lifting it up inside the parent's state so that its siblings could exploit that data... After a second thought, this doesn't seem ā€˜naturalā€˜.

I think it's actually OK. Think Fetch is like an input.

The mistake was in trying to set the state in the render phase. Instead you could call this.props.onChange() with data from AJAX callback in Fetch, and render null from it. Then, in App, you would have a change handler that sets the state, just like you did.

do we also have access to handleChange/onChange inside any React Native component or is it tied to the DOM?

Not sure what you mean. I suggest you to have Fetch accept onChange as a custom prop, and then have it call it when the data is fetched.

Here's an example of the second approach I was referring to: https://codesandbox.io/s/j7jlyp07l9

It's very close to what you did originally, but without the state duplication or the update-state-during-render issue.

Hope this helps!

Okay! I got it now.

I thought you were referring to the handleChange method used here and I didn't get why.

The custom onFetch prop is exactly what I was trying to achieve, but naively doing it inside the render phase. Plus I like this approach because it's very similar to what I have now so I'll be able to incrementally adopt it without breaking anything.

It definitely solved my problem, thank you!

Happy to help!

Was this page helpful?
0 / 5 - 0 ratings