react icon indicating copy to clipboard operation
react copied to clipboard

Bug: Conditionally rendering a lazy loaded component only after the parent node is attached causes infinite loop

Open danhorvath opened this issue 1 year ago • 9 comments

React version: 18.3.1 and 19.0.0-rc-b57d2823-20240822

Steps To Reproduce

  1. Create a component that renders the children inside a <div>, but only after it has obtained reference to that div (via putting the div node into a state)
  2. Pass a lazy loaded component as children

So basically something like:

import { Suspense, lazy, useState } from 'react';

const LazyLoadedComponent = lazy(
  () =>
    new Promise((resolve) => {
      setTimeout(
        () =>
          resolve({
            default: () => <div>Lazy loaded component</div>,
          }),
        500,
      );
    }),
);

const RenderAfterMount = (props) => {
  const [node, setNode] = useState(null);

  return <div ref={setNode}>{node && props.children}</div>;
};

export const App = () => (
  <Suspense>
    <RenderAfterMount>
      <LazyLoadedComponent />
    </RenderAfterMount>
  </Suspense>
);

Link to code example: https://codesandbox.io/s/vibrant-murdock-jpnznd?file=/src/App.js:542-561

The current behavior

Runtime error due to an infinite loop.

The expected behavior

The lazy loaded component is rendered.

danhorvath avatar Aug 02 '24 14:08 danhorvath

It may not be a perfect solution, but you need to try the code below I modified.

Modifyed Code

import { Suspense, lazy, useEffect, useRef, useState } from "react";
import "./styles.css";

const LazyLoadedComponent = lazy(
  () =>
    new Promise((resolve) => {
      setTimeout(
        () =>
          resolve({
            default: () => (
              <div style={{ background: "green" }}>Lazy loaded component</div>
            )
          }),
        500
      );
    })
);

export default function App() {
  return (
    <div className="App">
      <h1>Hello CodeSandbox</h1>
      <Suspense>
        <RenderAfterMount>
          <LazyLoadedComponent />
        </RenderAfterMount>
      </Suspense>
    </div>
  );
}

const RenderAfterMount = (props) => {
  const [node, setNode] = useState(null);
  const divRef = useRef(null); // This ref references to <div> component.

  // Called when the node state changes.
  useEffect(() => {
    if (divRef.current && !node) {
      setNode(divRef.current);
    }
  }, [node]);

  return <div ref={divRef}>{node && props.children}</div>;
};

MTtankkeo avatar Aug 03 '24 02:08 MTtankkeo

Hey bro, is it still not resolved?, or are you trying?

MTtankkeo avatar Aug 04 '24 01:08 MTtankkeo

You might need to update RenderAfterMount to:

const RenderAfterMount = (props) => {
  const [node, setNode] = useState(null);

  const popluateNode = (element) => element !== node && setNode(element);

  return <div ref={popluateNode}>{node && props.children}</div>;
};

behnammodi avatar Aug 04 '24 19:08 behnammodi

Thanks both, but these workarounds don't solve my issue. In my usecase I need to know about whether the parent element has been attached to the dom. Also, I believe that this is a bug, so i'm not looking for workarounds, I'm hoping to bring attention to it and get it fixed in react.

In short, my usecase is a workaround for https://github.com/facebook/react/issues/23301, where we

  1. first render out our dialog element without any content rendered inside it (and takes the focus)
  2. and then once the dialog element has been attached (and took the focus) we immediately render out it its children that might have autofocus set on them (which would otherwise not be respected inside native dialogs, see the issue in the link).

danhorvath avatar Aug 05 '24 07:08 danhorvath

I managed to reproduce the same issue by suspending on a simple promise instead of a lazy loaded component in the render cycle. Here's a sandbox for it: https://codesandbox.io/s/happy-williams-9yjs4p?file=/src/App.js

import { Suspense, useState } from "react";

function wrapPromise(promise) {
  let status = "pending";
  let response;

  const suspender = promise.then(
    (res) => {
      status = "success";
      response = res;
    },
    (error) => {
      status = "error";
      response = error;
    }
  );

  const read = () => {
    switch (status) {
      case "pending": {
        throw suspender;
      }
      case "error": {
        throw response;
      }
      default: {
        return response;
      }
    }
  };

  return { read };
}

const promise = wrapPromise(
  new Promise((resolve) =>
    setTimeout(() => {
      resolve("hello");
    }, 2000)
  )
);

const SuspendableComponent = () => {
  const [divNode, setDivNode] = useState(null);

  // removing the condition unbreaks the render
  if (divNode) {
    console.log(promise.read());
  }

  return <div ref={setDivNode} />;
};

export default function App() {
  return (
    <Suspense fallback={<div>loading...</div>}>
      <SuspendableComponent />
    </Suspense>
  );
}

danhorvath avatar Aug 05 '24 11:08 danhorvath

Thanks both, but these workarounds don't solve my issue. In my usecase I need to know about whether the parent element has been attached to the dom. Also, I believe that this is a bug, so i'm not looking for workarounds, I'm hoping to bring attention to it and get it fixed in react.

In short, my usecase is a workaround for #23301, where we

  1. first render out our dialog element without any content rendered inside it (and takes the focus)
  2. and then once the dialog element has been attached (and took the focus) we immediately render out it its children that might have autofocus set on them (which would otherwise not be respected inside native dialogs, see the issue in the link).

So it does not need to use setState to accomplish your goal right ?

hungcung2409 avatar Aug 06 '24 02:08 hungcung2409

It does need useState. I need a reference to the dialog element and my code needs to react to changes to that ref.

danhorvath avatar Aug 06 '24 08:08 danhorvath

If I understand correctly, you can use useRef to access to the dialog element.

const RenderAfterMount = (props) => {
  const ref = useRef(null);
  const cb = useCallback((node) => {
    ref.current = node;
    alert("attached");
  }, []);

  return <div ref={cb}>{props.children}</div>;
};

When your lazyComponent mounts, it will call the cb

https://codesandbox.io/p/sandbox/amazing-chaplygin-j5xszv

hungcung2409 avatar Aug 06 '24 08:08 hungcung2409

My component needs to re-render when the reference changes, therefore I need a useState. The code example in the bug description is a simplified version of my usecase.

danhorvath avatar Aug 06 '24 08:08 danhorvath

Any update on this issue?

jtbandes avatar Feb 07 '25 21:02 jtbandes

You need to wrap props.children in Suspense:

<RenderAfterMount>
  <Suspense>
    <LazyLoadedComponent />
  </Suspense>
</RenderAfterMount>

Since you only have Suspense above RenderAfterMount, RenderAfterMount will unmount once we suspend thus loosing all state. After the lazy component is resolved, it'll mount again but with initial state i.e. render no lazy component, get a ref, render again and suspend. This cycle will go on forever.

eps1lon avatar Feb 09 '25 11:02 eps1lon

That works, although if I understand correctly it means that it's not possible the whole of RenderAfterMount in this usecase.

danhorvath avatar Feb 18 '25 08:02 danhorvath