preactjs / preact / 6673774262 / 1

99%

master: 100%

push

github

Merge pull request #4172 from preactjs/improve-place-child

# Improve place child

When unmounting DOM, we need to move `oldDom`/`_nextDom` past the unmounted DOM so the diff can continue by comparing the next VNodes to DOM nodes that are mounted. We did this in the unmount loop but we missed a couple other places where this needs to happen as well.

Let's say we have the following components:
```jsx
function Conditional({ condition }) {
  return condition ? (
      <>
        <p>2</p>
        <p>3</p>
      </>
    ) : null;
  }

function App() { return <>... <Conditional/> ... </>; }
```

Here, Conditional will switch from a group of DOM nodes and `null`. When doing this, diffChildren would hit our null placeholder code which does update oldDom past the unmounted DOM, but it doesn't update `_nextDom`, so `App` wouldn't see this move and try to continue diffing from an unmounted DOM node.

Further, even if we set `_nextDOM`, diffChildren on App wouldn't see it because `Conditional` returned `null`. Since Conditional returned `null`, it would return `null` for `newDom`,  meaning diffChildren on App would skip over all the code that was previously under a `if (newDom)` check, including the code that reads `_nextDom`. This PR also fixes that issue so even if a component returns `null`, we update the skewed index, and will read `_nextDom` if necessary.

Also, the test "should efficiently unmount & mount components that conditionally return null with null first child" contains an additional edge case where Conditional switching to null, matches it's previous null first child. We need to ensure we detect we still need to move oldDom and update `_nextDom`.

Previously we attempted to handle all of these cases by checking `oldDom.parentNode !== parentDom` in `placeChild`. However, in our entire test case, this was only occurred when oldDom was unmounted. So this update prevents unmounted oldDom from ever reaching `placeChild` removing... (continued)

1170 of 1199 branches covered (0.0%)

693 of 697 relevant lines covered (99.43%)

852.87 hits per line