Preact: DOM elements are orphaned when rendered through a wrapping component

[m4rw3r]

Using Preact 10.0.0-beta.1, alternating between two different elements in a component being wrapped by another component without its own element will cause DOM elements to be orphaned when the wrapping component is replaced with another component, rendering the new component below the orphaned element.

This specific behaviour was observed when porting an application using react-redux and connected-react-router, but we have managed to produce a small example.

Example

import { render }   from "preact";
import { useState } from "preact/hooks";

// Plain component
const B = () => <p>This is b</p>;

// Component with state which swaps its returned element type
const C = () => {
  const [c, setC] = useState(false);

  // Trigger rerender, without this it will not produce the bug
  if( ! c) {
    setC(true);
  }

  // Important that these two elements are of different types,
  // if both are <p /> or <div /> it will not produce the bug
  return c ? <div>data</div> : <p>waiting</p>;
};

// Some wrapper component without its own element,
// could be a higher-order component like connect() or similar
const WrapC = () => <C />;

// Wrapper which swaps between components
const A = () => {
  const [a, setA] = useState(false);

  // Swap to B after we have let C rerender its <div />.
  // If we let C render its <div /> it will somehow end
  // up being an orphan.
  setTimeout(() => setA(true));

  // If we use a plain C here it will work
  // It will also work if we do not swap out C
  return a ? <B /> : <WrapC />
};

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

Function-components and hooks are used for conciseness, Component will also exhibit the same behaivour. The above code needs a small modification with useEffect to prevent infinite loops in React, but will otherwise produce the expected result there.

Expected behaviour

<C /> is rendered first, with its <p>waiting</p>, then gets immediately replaced with <div>data</div>. Then <C /> gets removed along with its <div>data</div> and <B /> rendered in its place with <p>this is b</p>.

The resulting visible page should only have the text "this is b".

Observed behaviour

<C /> is rendered first, with its <p>waiting</p>, then gets immediately replaced with <div>data</div>. Then <C /> gets removed but its <div>data</div> remains and <B />'s <p>this is b</p> is rendered underneath.

The resulting page has two paragraphs, "data", followed by "this is b" on its own line.

[JoviDeCroock]

Hey @m4rw3r,

Thanks for the amazing reproduction and detailed explanation. I'll look into this tonight and try to respond with more information as soon as I have any.
As of right now I think this could be related to another bug that got reported, so I'll test it with my fix branch first.

[andrewiggins]

Here is a codesandbox for anyone curious: https://codesandbox.io/s/m3k2qjvw8x

Took a look and #1617 does not fix this bug :/. My assessment is below. @marvinhagemeister, this may interest you as it is somewhat related to #1605.

To quickly summarize @m4rw3r excellent description above (thanks btw, helped a lot!):

1. initial render => <p>waiting</p>
2. setC => <div>data</div>
3. Expected: setA => <p>this is b</p>
4. Actual: setA => <div>data</div><p>this is b</p>

The VNode tree before the setC call is like this:

A
    WrapC (_dom: <p>)
        C (_dom: <p>)
            <p>waiting</p>

Once setC is called, the VNode tree becomes:

A
    WrapC (_dom: <p>)
        C (_dom: <div>)
            <div>data</div>

The diffing that happens because of the C setState call doesn't update it's parent's VNode _dom reference (this diff begins in forceUpdate). So when setA is called and A changes it the type of its render return (<WrapC /> to <B />), we try to unmount WrapC. When unmounting WrapC, the skipRemove logic in unmount just unmounts the <p> tag so the <div> remains.

Not yet sure on the best approach to fix this, but I imagine the discussion in #1605 is related since it is a somewhat similar problem (they both need info outside of the current diffing component to be correct)