Coverage remained the same at 100.0% when pulling d9547e3 on Almo7aya:fixReconciliation into 6fac51e on developit:master.

Hi @Almo7aya! This is great! Thanks for taking the time to contribute a fix 🎉

Could you add test case so we don't regress this in the future?

Related: #1440

@andrewiggins I'm stuck with writing a test case for this, the saveFocus function does not let me check for moving children, it saves the focused child and refocused it after rendering, so I can't detect if the children moved or not. Have any idea how to do this? :)

@Almo7aya Let's use the code from #1343 as a test case 🎉

it('should not re-render when a component returns undefined', () => {
  let Dialog = () => undefined;
  let updateState;
  class App extends Component {
    constructor(props) {
      super(props);
      this.state = { name: '' };
      updateState = () => this.setState({ name: ', friend' });
    }

    render(props, { name }) {
      return (
        <div>
          <Dialog />
          <h1 class="fade-down">Hi{name}</h1>
        </div>
      );
    }
  }

  render(<App />, scratch);
  clearLog();

  updateState();
  rerender();

  // We don't log text updates
  expect(getLog()).to.deep.equal([]);
});

EDIT: I can also confirm that this fixes #1343. Just checked it out locally 👍

@marvinhagemeister Sorry!, but even this doesn't work, it always passes. The problem is children reconciler append children at the end of the parent not in their correct position this cause starting the animation, I cannot detect if the children were appended at the end or at their correct position using insertBefore 🤦‍♂️!!

@Almo7aya are you sure? The test only passes with the changes from your PR. It fails in master.

@marvinhagemeister I think I have some problems with my testing environment if you are sure that this fails in master and passes with this PR I'll commit your test case anyway?!

@Almo7aya Good idea, we can always check if it runs properly in Travis here 👍

Just check it with travis it did not fail with the previous changes
https://travis-ci.com/Almo7aya/preact/builds/105919357#L1317
https://github.com/Almo7aya/preact/blob/test-tesing/src/diff/children.js#L32

@Almo7aya Oh right, I found why it always passes. Our test logger needs to be set up for it to work. We usually do it in the test runners before hook:

import { logCall } from '../_util/logCall';

describe("render()", () => {
  // ...

  before(() => {
    logCall(Element.prototype, 'appendChild');
    logCall(Element.prototype, 'insertBefore');
    logCall(Element.prototype, 'removeChild');
    logCall(Element.prototype, 'remove');
  });

  // ...
});

Only once it's attached to a prototype it will start logging DOM operations.

@marvinhagemeister That's great, thank you 🎉!

@marvinhagemeister I think it's ready now. Thank you for your help, I didn't know about loggers functions in the test utils. Again thank you!🎉.