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

Calling setState in render causes infinite loop #5591

Closed
mikkoh opened this issue Dec 3, 2015 · 26 comments
Closed

Calling setState in render causes infinite loop #5591

mikkoh opened this issue Dec 3, 2015 · 26 comments

Comments

@mikkoh
Copy link

@mikkoh mikkoh commented Dec 3, 2015

This may seem really silly to do to call setState in render. However it's possible for this to happen if a component has a callback which is being called immediately during render and in your callback handler you call setState.

@jimfb
Copy link
Contributor

@jimfb jimfb commented Dec 3, 2015

In general, this should never happen; you should not be doing anything remotely related to modifying any component's state within a render function.

I'm curious what your use case is, for having a callback which does a setState; mind sharing?

@jimbolla
Copy link

@jimbolla jimbolla commented Dec 3, 2015

I ran into this by accident once... I had onClick={this.setState({inProgress: true})} instead of onClick={() => this.setState({inProgress: true})}. It would be nice if React detected the problem and threw an error "setState detected in render() of SomeComponent" instead of going into the infinite loop.

@mikkoh
Copy link
Author

@mikkoh mikkoh commented Dec 3, 2015

A video player is playing. I have an Component which sets styles on the scrub bar. When the Component that sets styles updates it fires a callback. (there's a good reason for this but it's beyond explaining in a simple issue) I was listening to this callback and calling setState to notify my component changes happened.

In general setState isn't evaluated until before the next "render" but in this case setState calls immediately which is divergent behaviour from the normal usage of setState.

@jimfb
Copy link
Contributor

@jimfb jimfb commented Dec 3, 2015

If I'm understanding your flow correctly, I still don't see how/why a setState would be occurring within a component's render() function. Do you just mean within a single reconciliation cycle? Keep in mind that render() is not recursive (that's a common miss-conception; the child's render function is not called during the parent's render function).

@mikkoh wrote:

When the Component that sets styles updates it fires a callback.

I assume you're using componentDidUpdate for this? Can you provide a simple JSFiddle example/testcase that demonstrates the error you're running into?

@mikkoh
Copy link
Author

@mikkoh mikkoh commented Dec 3, 2015

This is an oversimplification of the code but:

handleSomething(value) {
  this.setState({
    something: value
  });
}

render() {
  return <Foo 
    someProp={this.props.someValue}
    onCallback={this.handleSomething} 
  />;
}

When Foo receives props it calls onCallback which in turn calls handleSomething. Which will cause render to be called immediately. Then once again handleSomething gets called then render and so on.

Again this is an oversimplification and what Foo might be doing is dumb but react still shouldn't call render immediately after setState in the same reconciliation cycle.

@sophiebits
Copy link
Collaborator

@sophiebits sophiebits commented Dec 5, 2015

Again this is an oversimplification and what Foo might be doing is dumb but react still shouldn't call render immediately after setState in the same reconciliation cycle.

The fact that we support calling setState in componentDidMount or componentDidUpdate is not an accident – doing so in order to trigger an update is sometimes useful.

@jcehle
Copy link

@jcehle jcehle commented Dec 18, 2015

@mikkoh Did you find a workaround?
just came across this exact issue..

@mikkoh
Copy link
Author

@mikkoh mikkoh commented Dec 21, 2015

I ended up implementing process.nextTick where the callback gets called. This is a workaround and not a fix.

@spicyj setState was not being called called in componentDidMount or componentDidUpdate but rather in a callback that resulted/was called because a component being rendered.

@sophiebits
Copy link
Collaborator

@sophiebits sophiebits commented Dec 21, 2015

@mikkoh Can you provide a specific code example here? I'm not sure I understand exactly what your issue was.

@mikkoh
Copy link
Author

@mikkoh mikkoh commented Dec 22, 2015

@spicyj I posted a snippet earlier. So below when Foo renders it will call this.handleSomething via props.onCallback. Now it's unfortunate cause I can't remember remember at which stage of Foo's render lifecycle props.onCallback gets called (render, componentWillMount, etc) But basically the simple act of Foo being rendered called props.onCallback.

handleSomething(value) {
  this.setState({
    something: value
  });
}

render() {
  return <Foo 
    someProp={this.props.someValue}
    onCallback={this.handleSomething} 
  />;
}
@sophiebits
Copy link
Collaborator

@sophiebits sophiebits commented Dec 22, 2015

I'm still failing to understand why what you're doing causes an infinite loop but that you say adding a process.nextTick alleviates the problem. In either case, if you call setState repeatedly, the component should continue to rerender.

If you can post a working code example (in jsbin, for example) that shows the problem, I'm happy to take another look – but as is since I can't understand what you're doing this is not very actionable for me. I don't understand what the intention of your code is if you're trying to update the parent's state whenever a child renders. Wouldn't you expect that to cause an infinite loop?

@dakom
Copy link

@dakom dakom commented Oct 1, 2017

I realize this is an old ticket and there might be more idiomatic ways of doing things in React now - but I just hit an issue which is related, so figured here's as good a place as any to comment ;)

What's the best way to handle situations where you need to measure after the component has been mounted? I'd prefer to avoid ref because there's no need to actually call the component. Also, when dealing with custom renderers I'm not sure how to implement ref and furthermore - CSS tricks like 'visible' may not apply.

Here's an example of what I mean:

class MyComponent extends React.Component {
    private instanceWidth:number;

    constructor(props) {
        super(props)
        this.state = {}
    }
   
    componentDidMount() {
        this.setState({
            instanceWidth: this.instanceWidth
        })
    }

    render() {
        return <SomeElement 
            onAdded={instance => this.instanceWidth = instance.width}
            x={this.state.instanceWidth === undefined ? 0 : window.innerWidth - this.state.instanceWidth}
            {...props}
        />
    }
}

The issues I have with this approach are:

  1. It's bordering on like setState within render. Not really since it will only happen the first time via cDM, but still... feels icky with that extra private var and... I dunno... just feels odd?

  2. There's going to be a blip where the wrong position is truly rendered. The only way around this I can think of is to manage it via visual tricks like setting opacity.

I'd love to hear thoughts on why this is or isn't the right way to do it. Thanks!

@gaearon
Copy link
Member

@gaearon gaearon commented Oct 2, 2017

  1. It is absolutely fine to call setState from componentDidMount if you have to read something from the DOM. It's not supposed to be pure. In fact the measurement use case is one of the reasons it exists.

  2. React was designed with this use case in mind. There shouldn't be a blip because React processes setState from componentDidMount synchronously to avoid this problem.

@sunil-sandhu
Copy link

@sunil-sandhu sunil-sandhu commented Mar 30, 2018

So so late to the party here but @sophiebits's comment about componentDidMount has just brought an end to 3 hours of debugging - thank you!

@spyx08
Copy link

@spyx08 spyx08 commented May 4, 2018

I've a strange issue.
I have big forms who are rendering with state dropdowns. And i've to check when populating the dropdowns if a value exist. And if she exist, update the child dropdown with the right values.
I store all changements in a variable in componentDidMount state I update each dropdown who are stored in my variable. but it cause an infinite loop...

@poxrud
Copy link

@poxrud poxrud commented May 8, 2018

@spyx08 can we see a code sample?

Here's what will cause an infinite loop, assuming you're not doing any state change checks:

  • setState inside componentWillUpdate
  • setState inside componentDidUpdate
  • setState inside render (this is usually accidental)
  • setState inside getSnapshotBeforeUpdate

Make sure you're not doing any of the above and you shouldn't see an infinite loop.

@lisadesouza89
Copy link

@lisadesouza89 lisadesouza89 commented May 22, 2018

I am also facing a similar issue as @mikkoh : where I'm passing down a parent function through props, to be called from a button inside a child component; which is going into an infinite loop.

handleSomething(value) {
this.setState({
something: value
});
}

render() {
return ;
}

where I am only calling this.props.onCallback from a button inside my child.

Help!

@anzome
Copy link

@anzome anzome commented May 22, 2018

@lisadesouza89, all depends on how exactly your are calling the this.prrops.onCallback inside the Button. Could you provide this information?

@lisadesouza89
Copy link

@lisadesouza89 lisadesouza89 commented May 22, 2018

Parent component:

constructor{...
this.initModal = this.initModal.bind(this);
}
initModal = (localRecord) => {
		console.log("Called method with record:"+JSON.stringify(localRecord));
		this.setState({record:localRecord,modalIsOpen:true});		
	};
return{
<Child1 mRecord={mRecord} init={this.initModal}/>
}

Child1:

render{
            <span>
                    <Child2 mRecord={this.props.mRecord} init={this.props.init}/>
                </span>
            </div>
            );
}

Child2:

render{
<div>
       <button onClick={this.props.init(this.props.mRecord)}/>
</div>
}
@anzome
Copy link

@anzome anzome commented May 22, 2018

You shouldn't call init within onClick declaration. Since you know mRecord within parent you could bind it to initModal inside parent's constructor.

//in parent
this.initModal = this.initModal.bind(this, mRecord);
// in child 2
<button onClick={this.props.init}/>
@lisadesouza89
Copy link

@lisadesouza89 lisadesouza89 commented May 22, 2018

There are multiple Child1 and Child2 elements with different values, so no, I don't know mRecord in the parent.
But I figured out my issue: I should have
<button onClick={()=>this.props.init(this.props.mRecord)}/>
instead of
<button onClick={this.props.init(this.props.mRecord)}/>

@Ashutosh21Nigam
Copy link

@Ashutosh21Nigam Ashutosh21Nigam commented May 23, 2018

how to use react native flex radio button inside for loop

@markuspxpx
Copy link

@markuspxpx markuspxpx commented Nov 16, 2018

I have a simple solution, compares the state before you change, it worked for me.
I just "setState" the state if the new state is different than the current.

@ashwithags
Copy link

@ashwithags ashwithags commented Mar 27, 2019

I have developed a picker for setting day and night. I getting the current time for a picker value from an API and trying to set with setState, but every time it will display 'day' as a value. I want it should display based on the value passed. How can I solve the issue here

 this.state= {
    time: null
}

  timeMode = (Currenttime) => {   //here if the passed value is 'night' then it should set the value to 
night.  Then when clicking on picker it should be both values in dropdown

  this.setState({ time: Currenttime }); // this causes infinite loop...
  return (
    <View style={Styles.paddingStyle}>
        <Picker
            selectedValue={this.state.time}
            mode="dropdown"
            onValueChange={(value) => this.setState({time: value})}   >
            {this.DropdownValues()}
        </Picker>
    </View>
   )
}

DropdownValues = () => {
const time = [
  'day',
  'night'
];
return time.map(data => {
  return <Picker.Item label={data} value={data} />;
});
};
@seelasudheer
Copy link

@seelasudheer seelasudheer commented Mar 29, 2020

I ran into this by accident once... I had onClick={this.setState({inProgress: true})} instead of onClick={() => this.setState({inProgress: true})}. It would be nice if React detected the problem and threw an error "setState detected in render() of SomeComponent" instead of going into the infinite loop.

#18424

@roy-shantanu
Copy link

@roy-shantanu roy-shantanu commented May 23, 2020

I was just passing my lockdown times learning react. I came into a similar situation. So react does go into an infinite loop when I call setState directly inside the render, but react is smart enough to detect it and stops, throwing and error that something dumb has happend. But if I do a dynamic import and call setState inside then(), it keeps on rendering, and it got my CPU fans roaring... any reasons why it doesn't detect "Maximum update depth exceeded" when setState is called in such a way?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
You can’t perform that action at this time.