componentWillUnmount not firing for a root component that renders null.

[joeldenning]

Somewhat related to #392, #373, and #536. See https://jsfiddle.net/7d3tojb1/ for repro -- it uses preact@7.2.0.

The problem is that if you have a root component that renders null, then preact does not call componentWillUnmount on that component.

class Nothing extends preact.Component {
  render() {
  	console.log('rendering Nothing')
  	return null;
  }
  componentWillUnmount() {
  	calledCWU = true;
  	console.log('componentWillUnmount Nothing')
  }
}
const nothingRoot = preact.render(preact.h(Nothing, {}), document.body);

// This should unmount the component
preact.render('', document.body, nothingRoot);

[joeldenning]

Seems related to https://github.com/developit/preact/blob/master/src/vdom/diff.js#L81-L95 -- rendering null seems to create a text node which goes into that if statement when things are being unmounted. The return dom short circuits the idiff function such that componentWillUnmount is never called.

[developit]

Ah interesting - I had thought this was related more to PFC's, you're right it's because of the early bailout for text nodes. Seems like we just need to move the component handling on L98 to happen before the Text node shortcircuiting?

[joeldenning]

I tried moving the component handling on L98 above the Text node shortcircuiting, but it didn't seem to help. In my example case, vnode === null, which then turns into vnode = '' on https://github.com/developit/preact/blob/master/src/vdom/diff.js#L77. So the if (isFunction(vnode.nodeName)) check on L98 isn't truthy and the buildComponentFromVNode function is not getting called.

This is my first venture into the preact source so I am still figuring out what a solution would look like, but I'll keep investigating.

[developit]

Ah yes, sorry my brain was still associating this with components. Maybe rendering a Comment node for component roots would fix this because it'd skip the shortcircuiting? You'd hit the comment node shortcircuit for component A -> B, but that'd be triggered from renderComponent(), which handles unmounting mismatched components by itself.