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

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

Closed
danhorvath opened this issue Aug 2, 2024 · 12 comments

Comments

@danhorvath
Copy link

danhorvath commented Aug 2, 2024

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 danhorvath added the Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug label Aug 2, 2024
@danhorvath danhorvath changed the title Bug: Conditionally rendering a lazy loaded element only after the parent node is attached causes infinite loop Bug: Conditionally rendering a lazy loaded component only after the parent node is attached causes infinite loop Aug 2, 2024
@MTtankkeo
Copy link

MTtankkeo commented Aug 3, 2024

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
Copy link

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

@behnammodi
Copy link
Contributor

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>;
};

@danhorvath
Copy link
Author

danhorvath commented Aug 5, 2024

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).

@danhorvath
Copy link
Author

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>
  );
}

@hungcung2409
Copy link

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 ?

@danhorvath
Copy link
Author

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

@hungcung2409
Copy link

hungcung2409 commented Aug 6, 2024

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

@danhorvath
Copy link
Author

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.

@jtbandes
Copy link

jtbandes commented Feb 7, 2025

Any update on this issue?

@eps1lon
Copy link
Collaborator

eps1lon commented Feb 9, 2025

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.

@danhorvath
Copy link
Author

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

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

6 participants