NotFoundError: Failed to execute 'removeChild' on 'Node'

[parisk]

We also ran into this issue on SourceLair, at the same time with #919 (same user).

Seems like rowContainer can be detached from term.element at some points 🤔.

https://github.com/sourcelair/xterm.js/blob/6e9d60d76d6cfb3d279f9e58499ba62ca6af479c/src/Renderer.ts#L129-L131

Details

• Sentry Error: https://sentry.io/share/issue/35323433302e333334303630393631/
• Browser and browser version: Chrome 60
• OS version: Windows 8
• xterm.js version: 2.9.2

[Tyriar]

It's removed and added again at the end of refresh so a partial layout isn't attempted. All the DOM manipulations are done while it's detached. This is meant to be the only point that it's detached though.

[parisk]

https://github.com/sourcelair/xterm.js/blob/6e9d60d76d6cfb3d279f9e58499ba62ca6af479c/src/Renderer.ts#L319-L321

So based on the lines below, it seems that the issue is:

1. Refresh happens, but at the end of it, there is no parent, so the element is not put back
2. Another refresh happens, but now while a parent element exists (for some reason), the rowContainer has not been put back into its place

Maybe we should perform a check for rowContainer as well, before removing or appending it.

[Tyriar]

After a closer look, I notice parent != Terminal.parent (at least i the sense is can be unset).

parent gets set if the terminal is attached to the DOM (ie. Terminal.element has a parentNode) and > 50% of the viewport is getting refreshed. I don't think a check for rowContainer would change the outcome, or it would hide the underlying bug.

Perhaps something in another class is messing with the DOM in an unexpected way?

[JoshuaKGoldberg]

Hey @Tyriar, the situation of another class messing with the DOM actually reproduces reliably now with hot module reloading in React applications. Here's a demo: https://codesandbox.io/s/xtermjs-dom-renderer-disposal-jsd75

useEffect(() => {
  if (!divRef.current) {
    return;
  }

  const terminal = new Terminal({
    rendererType: "dom"
  });

  terminal.open(divRef.current);
  terminal.write(`It has begun! ${Math.random()}`);

  return () => {
    // To cause this line to crash, modify some code in CodeSandbox.
    // That'll cause a hot module reload (HMR), which removes the
    // terminal's element *before* this disposal function is run.
    terminal.dispose();
  };
}, [divRef]);

I'd be happy to send a PR that, say, changes the DOM renderer's dispose() to only removeChild the intended elements if they're children of _screenElement?

[Tyriar]

@JoshuaKGoldberg yes please send a PR 🙂

[Tyriar]

👉 #2960