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

Possible Bug: interplay between reconcilliation algorithm and DOM manipulations via refs #20891

Closed
fast-reflexes opened this issue Feb 26, 2021 · 2 comments
Labels
Resolution: Stale Automatically closed due to inactivity Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug

Comments

@fast-reflexes
Copy link

Introduction

I'm working with a library that performs DOM manipulations via refs in useEffects after React has rendered.

Background

My understanding of the reconciliation algorithm is that it compares subsequently rendered virtual DOMs and only perform the necessary updates on the real DOM when necessary according to this comparison. For this reason, the following example will continue to show a div with a green background throughout subsequent rerenderings, even though the render method returns a div with a blue background:

export default function App() {
  const [num, setNum] = useState(10);
  const ref = useRef();

  useEffect(() => {
    console.log("effect ran");
    if (ref.current) {
      ref.current.style.backgroundColor = "green";
    }
  }, []);

  console.log("rendered");

  return (
    <div ref={ref} style={{ backgroundColor: "blue", padding: num + "px" }}>
      <button onClick={() => setNum((num) => num + 10)}>Rerender</button>
    </div>
  );
}

(Sandbox link: https://codesandbox.io/s/festive-leavitt-n44hs?file=/src/App.js)
React is simply unaware of the change done directly via the ref and so the comparison between subsequent virtual DOMs only indicate that the padding must be updated.

Problem

This is all fine when it comes to properties, but as far as children are concerned, the behaviour is slightly more obscure and inconsistent (perhaps due to children seemingly being something in between a prop on the parent element and distinct other elements?).

Consider the following toy example where a wrapper component performs an update (replaces 0s with 9s) on the content of its DOM children via refs after rendering. Remember that this is a toy example and I know that the results in this example can be acquired in a much simpler (better!) way in React.

export default function App() {
  const [num, setNum] = useState(10);

  return (
    <div>
      <WrapperComponent id="first">
        <span /*key={ Date.now() }*/>10 - {num}</span>
      </WrapperComponent>
      <WrapperComponent id="second">
        <span /*key={ Date.now() }*/>10 -</span>
        <span /*key={ Date.now() + 1 }*/>{num}</span>
      </WrapperComponent>
      <button onClick={() => setNum((num) => num + 10)}>Rerender</button>
    </div>
  );
}

const WrapperComponent = (props) => {
  const ref = useRef();

  console.log(props.id + ": render");
  console.log(props.children);

  useEffect(() => {
    if (ref.current) {
      ref.current.style.backgroundColor = "green";
      console.log(props.id + ": pre-alter");
      console.log(ref.current.innerHTML);
      for (let i = 0; i < ref.current.children.length; ++i) {
        let el = ref.current.children[i];
        if (el.children.length === 0)
          el.innerHTML = el.innerHTML.replaceAll("0", "9");
      }
      //ref.current.innerHTML = "Altered HTML"
      console.log(props.id + ": post-alter");
      console.log(ref.current.innerHTML);
    }
  });

  return (
    <div
      ref={ref}
      style={{ margin: "20px", color: "white", backgroundColor: "blue" }}
    >
      {props.children}
    </div>
  );
};

(Sandbox link: https://codesandbox.io/s/cranky-blackwell-xqbkx?file=/src/App.js)

Depending on the structure of the children, the results differ:

  • Setup 1: In the unaltered version above, render once and then clear the console. Click the button to render again and note that the logged children of the WrapperComponents contain the updated number correctly in the render printout. Then note that in the useEffect when inspecting the refs, React has changed the content of the second WrapperComponent in the real DOM to reflect the update in the child components in the virtual DOM whereas the child of the first has not been updated (see pre-alter printout). As a result, the changes (in the first WrapperComponent) are not reflected in the UI and the useEffect has nothing new to process.
  • Setup2: Comment out the for loop in the useEffect of the WrapperComponent and uncomment the row below it. Refresh the UI and press the button. With this version, React doesn't update the real DOM at all (can be seen in the pre-alter printout) even though the children are still correct in the render printout.
  • Setup3: Uncomment the key props in the spans and restore the for loop in WrapperComponent and comment out the row below it again. Update the UI and hit the button repeatedly and note that now it works perfectly. React updates the DOM as expected and this is reflected in both pre-alter printouts.

React version: 17.0

Steps To Reproduce

See previous section

Link to code example:

See previous section

The current behavior

See previous section

The expected behavior

  • Setup2: Given that the comparison between virtual DOMs indicate that only the content of the spans has changed, it is reasonable that React tries to update only that and bails out silently when it can no longer find those nodes in the real DOM. One could reason that React should reinsert them but I still think this behaviour is consistent.
  • Setup3: Comparisons of virtual DOMs indicate that completely new children are to be inserted in the WrapperComponents and so the old ones are thrown out and the new ones are added. Everything works as desired.
  • Setup1: The WrapperComponents only update the content of the spans and so React should be able to map content change in the virtual DOMs to these very same spans. It manages to do so for the second WrapperComponent but not the first, despite the changed content.

I would expect this behaviour to be more consistent and not depend on the structure of the children like this. My suspicion is that it seems to be related to how React maps virtual DOM nodes with mixed content (expressions and literal text) to real DOM nodes but I can't see how, where and why. Hopefully this is not a bug in which case I am sorry for wasting your time.

@fast-reflexes fast-reflexes added the Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug label Feb 26, 2021
Copy link

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!

@github-actions github-actions bot added the Resolution: Stale Automatically closed due to inactivity label Apr 10, 2024
Copy link

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!

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Apr 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution: Stale Automatically closed due to inactivity Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug
Projects
None yet
Development

No branches or pull requests

1 participant