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

Provide a way to detect infinite component rendering recursion in development #12525

Open
josh-degraw opened this issue Apr 3, 2018 · 20 comments

Comments

@josh-degraw
Copy link

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

  • Feature (possibly bug?)

What is the current behavior?

I've been trying out the new Context API in my project and it's awesome. However, in my haste to start using it, I managed to stumble into a situation where every time I would try and render a certain component which was making use of a few different contexts, the app would completely freeze, and the only thing that would let me get out of this error state was to forcefully kill the process via the chrome task manager.

Nothing would be logged to the console, the app would just completely freeze, and when I opened up the task manager and saw the CPU spiked up every time i would go to this component, and the only way I could stop it was to crash the tab.

I finally threw some console statements in and saw that it had just entered into an infinite loop between these providers. I managed to get the app to stop crashing, but I'm still unsure as to why exactly this was happening. I'm sure I was just using this API incorrectly somehow, but this was a very confusing problem to diagnose, and some error checking here would be incredibly useful

What is the expected behavior?

It would be very beneficial to have some sort of checks in place, similar to what happens with too many setState calls happening too closely when you call it from componentDidUpdate, for example. That way, instead of freezing everything up permanently, the app could at least crash and report some sort of information and help me realize where I'd gone wrong.

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

  • React 16.3.0
  • Chrome 65.0.3325.181
@gaearon
Copy link
Collaborator

gaearon commented Apr 3, 2018

Is there something specific to context here? I guess you'd have a similar problem if you rendered a component that recursively renders itself?

@awnigharbia
Copy link

awnigharbia commented Apr 3, 2018

@gaearon Is it the same issue I post it before two days? here?
#12515

@josh-degraw
Copy link
Author

Yeah that's basically what was going on, it was just a recursive call with no end condition, and I just didn't realize as I was writing it.

It's probably more that I was using HOC functions to do this, and those functions were recursively calling each other without me realizing.

I had set up a few different functions like this to combine common contexts:

const AppContextTypeContext = React.createContext('None');
const AppContextIdContext = React.createContext(0);

function AppContextProvider({ contextType, contextId, ...props }) {
    return (
        <AppContextTypeContext.Provider value={contextType}>
            <AppContextIdContext.Provider value={contextId} {...props} />
        </AppContextTypeContext.Provider>
    );  
}

with matching HOC functions for the context like this:

const withAppContext = WrappedComponent => function WithAppContext(props) {
    return (
        <AppContextTypeContext.Consumer>
            {contextType => (
                <AppContextIdContext.Consumer>
                    {contextId => <WrappedComponent contextType={contextType} contextId={contextId} {...props} />}
                </AppContextIdContext.Consumer>
            )}
        </AppContextTypeContext.Consumer>);
};

and I was combining them in multiple ways in multiple spots, and I guess I just ended up with the wrong combination of these functions that led to this issue.

So in short yeah, it's probably not specific just to contexts, but I could see many others easily running into this issue like I did.

@nickserv
Copy link

nickserv commented Apr 3, 2018

This does seem to be a duplicate of #12515, though it's been a helpful discussion so far.

@gaearon
Copy link
Collaborator

gaearon commented Apr 3, 2018

I'm just struggling to see how #12515 or this is different from just

function Child() {
  return <Child />;
}

or

function A() {
  return <B />;
}

function B() {
  return <A />;
}

@josh-degraw
Copy link
Author

josh-degraw commented Apr 4, 2018

Honestly I think it really is effictively just a more roundabout form of your second example. It's easy to see when it's like that, but it gets more difficult to diagnose when you have used 3 or 4 HOCs on top of otherwise unrelated components that then happen to call each other.

I guess what I'm requesting here is really just some sort of measure to detect this and throw an error, and break out of that loop somehow, so recursive issues such as this can be diagnosed easier, much like how you handle too many setState calls occurring too close together.

@nickserv
Copy link

nickserv commented Apr 4, 2018

I think the issue is technically unrelated to context, though it seems to be easier to run into with the new context API. Is there any built in safeguard to prevent too much recursion or would a check for this be easy to add at least for debugging purposes?

@gaearon gaearon changed the title Error messages for infinite context loops Provide a way to detect infinite component rendering recursion in development Apr 26, 2018
@nmain
Copy link

nmain commented Apr 26, 2018

What would be the desired solution here; keep track of how deep components have nested during rendering and abort with message when it's too deep? (200? 500?)

@nickserv
Copy link

I was originally thinking about warning when a component's child renders another instance of itself, but that could have confusing false positives. I guess setting a max depth is a good compromise, if it only warns in dev.

@anthonyhumphreys
Copy link

It’s not necessarily what you’re looking for, but I find the npm package “why did you update” to be useful in identifying unnecessary rerenders.

@giraffesyo
Copy link

This is basically the halting problem, a program cannot be created that will determine if another program will ever stop running. However, you can set some form of counter and after so many iterations halt a program.

I don't think it is a something that should be added as it's the same idea across all programming languages, if you keep calling a function infinitely, your program will run infinitely.

@stale
Copy link

stale bot commented Jan 10, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution.

@stale stale bot added the Resolution: Stale Automatically closed due to inactivity label Jan 10, 2020
@stale
Copy link

stale bot commented Jan 17, 2020

Closing this issue after a prolonged period of inactivity. If this issue is still present in the latest release, please create a new issue with up-to-date information. Thank you!

@stale stale bot closed this as completed Jan 17, 2020
@sahilrajput03
Copy link

sahilrajput03 commented Oct 5, 2020

//Custom hook_1
const useRenderCount = () => {
  // src: https://medium.com/better-programming/how-to-properly-use-the-react-useref-hook-in-concurrent-mode-38c54543857b
  const renderCount = useRef(0);
  let renderCountLocal = renderCount.current;
  useEffect(() => {
    renderCount.current = renderCountLocal;
  });
  renderCountLocal++;
  return renderCount.current;
};

//Custom hook_2 (We just need this, we'll call the other one implicitly).
const useStopInfiniteRender = (maxRenders) => {
  const renderCount = useRenderCount();
  if (renderCount > maxRenders)  throw new Error('Infinite Renders limit reached!!');
};

And using that custom hook in a react component like,

  const App () => {

  useStopInfiniteRender(10); //This works great for me, but it'll throw error at 11th render. 
  // You may set it to your need like 10000 is a good number.

  //...more code..
  }

@eps1lon
Copy link
Collaborator

eps1lon commented Jan 20, 2021

Re-opening since it came up again in #20629

@eps1lon eps1lon reopened this Jan 20, 2021
@stale stale bot removed the Resolution: Stale Automatically closed due to inactivity label Jan 20, 2021
@fwouts
Copy link

fwouts commented Sep 28, 2021

I find it surprising that React gets stuck indefinitely when it encounters infinite recursion whereas JS itself prevents it.

For example, the following crashes almost immediately with Uncaught InternalError: too much recursion on Firefox:

const f = () => f();

f();

On the other hand, the following just freezes the browser tab:

const F = () => <F />;

ReactDOM.render(<F />, document.getElementById("root"));

I wonder if there's a way to make React's behaviour consistent with plain function calls somehow? Surely nobody uses React trees with more than 10,000 levels of depth :)

@stale
Copy link

stale bot commented Jan 8, 2022

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

@stale stale bot added the Resolution: Stale Automatically closed due to inactivity label Jan 8, 2022
@gla23
Copy link

gla23 commented Oct 4, 2022

I struggled to debug my infinite loop because the chrome tab would crash (which took ages to close and reload) and the component tree wouldn't render (to display why the loop was happening). A helpful tip to get round these two issues is to use the following:

if (Math.random() < 0.2) return "infinite loop ended here";

@stale stale bot removed the Resolution: Stale Automatically closed due to inactivity label Oct 4, 2022
@NoamAnisfeld
Copy link

Just bumped into this, too - the cause of the issue is ridiculously simple but it's very hard to debug due to the unclear freeze.

Sure as mentioned, theoretically there's no way to detect an infinite loop. But in practice you could set it to stop and output an error when the recursion depth reaches some very high value - just like vanilla JS does. Or if stopping is not acceptable, at least output a warning when in development and strict mode.

@PetrusAsikainen
Copy link

PetrusAsikainen commented Mar 12, 2024

It would also probably be possible to detect if the same component is rendered within itself with the same props (and no context providers in between) - potentially with a recursion depth check as well, if the props comparison would be too expensive alone.

Was also surprised today to find that this didn't cause a warning or stack overflow of any kind, just a freeze (with findStrictRoot at 80% in the profiler).

One could also argue that if your component tree is, say, 10000 nodes deep, your app probably isn't working very well anymore...

rickhanlonii added a commit that referenced this issue Mar 24, 2024
Prevent issues like #12525 from
closing due to inactivity.
EdisonVan pushed a commit to EdisonVan/react that referenced this issue Apr 15, 2024
Prevent issues like facebook#12525 from
closing due to inactivity.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests