Diff algorithm problems when wrapping the children property

[nilzona]

I have reproduced an example below that generates a list of elements based on some input. If I create a Wrapper component that passes the elements down through the children prop it seems to break the diffing algorithm when re-rendering with preact.render on the same element in the dom tree using replaceNode.

const PreactComponent = ({data}) => {
  const elements = data.map(i => <span>{i}</span>);
  const Wrapper = ({children}) => <div>{children}</div>;
  return (
      <Wrapper>{elements}</Wrapper>
  );
}

preact.render(<PreactComponent data={data} />, container, container.firstChild />);

If I don't use the Wrapper component and just put it in a normal div it works as expected.

const PreactComponent = ({data}) => {
  const elements = data.map(i => <span>{i}</span>);
  return (
      <div>{elements}</div>
  );
}

preact.render(<PreactComponent data={data} />, container, container.firstChild />);

Reproduction

https://stackblitz.com/edit/preact-starter-template-ymdqam

Steps to reproduce

Click on the button "Add data"

Expected Behavior

New elements should gracefully be added to the dom tree when clicking the button.

Actual Behavior

Every second click the preact component disappears.

[JoviDeCroock]

Thrashing components inside a render function is generally seen as a bad-practice (and performance hog), I think preact/debug warns for this.

 const Wrapper = ({children}) => <div>{children}</div>;
const PreactComponent = ({data}) => {
  const elements = data.map(i => <span>{i}</span>);
  return (
      <Wrapper>{elements}</Wrapper>
  );
}

preact.render(<PreactComponent data={data} />, container, container.firstChild />);

That being said I'll look into the core issue when I can, the above is more of a piece of information for anyone bouncing on this issue.

[andrewiggins]

I think this is the same issue as #2496. Try only use the third parameter of render when hydrating SSR'ed DOM (or better yet, use the hydrate method). Subsequent rerenders should not include the 3rd parameter. This is a change from Preact 8. I think our doc page on differences from Preact 8 covers this.

Edit: scratch my earlier advice. I need to dig deeper into this

[developit]

There are two things at play here: the third replaceChild argument to render is generally something that should be avoided - it was kept so that niche rendering cases from Preact 8 could transition seamlessly to Preact 10. Removing that argument fixes the demo: https://stackblitz.com/edit/preact-starter-template-m5h6yg

The second issue is a bug caused by the thing @JoviDeCroock mentioned: when using replaceChild, the renderer is not currently capable of "swapping" the root component or element when its type changes. While it might not look like <Wrapper> is changing every time renderWithPreact() is called, it actually is - each time PreactComponent is invoked, it creates an entirely new Wrapper() function, which it then passes back as the type of the root JSX element. Technically this requires that the renderer destroy and recreate Wrapper and everything it contains, however Preact currently cannot do this for root components (that's a bug).

On the other hand, it's worth noting that even with the above bug fixed, rendering performance would be extremely poor because every update destroys and recreates the entire UI. The result would be slower than function render(data){dom.innerHTML=App(data)}. It would also have all sorts of issues with input state, video playback, etc.

[JoviDeCroock]

Going to close this as won't fix and that we now throw an error when component thrashing is part of a component.