preactjs / preact-compat

ATTENTION: The React compatibility layer for Preact has moved to the main preact repo.
http://npm.im/preact-compat
MIT License
950 stars 148 forks source link

createPortal replacing the destination node instead of apending #515

Closed y-lohse closed 4 years ago

y-lohse commented 5 years ago

Hi, and thanks for working on preact-compat!

We just discovered that if the component passed to createPortal is empty or falsy, the destination DOM node is cleared. Here's a minimal reproduction : https://codesandbox.io/s/9qyr3klq7r Replacing <Void /> by <NotVoid /> reverts to what we think is a normal behavior.

This doesn't see in line with how react behaves : https://codesandbox.io/s/new

Is this the expected behavior in preact-compat or is it a bug?

developit commented 5 years ago

Hi Yannick! It looks like a bug. Preact is having trouble rendering into the body in your example because there is already an application rendered there.

It looks like the issue is that preact-compat's Portal implementation uses the destructive version of render(), so it replaces the contents of <body> entirely rather than adding a new Element: https://github.com/developit/preact-compat/blob/master/src/index.js#L143-L151

I think we can fix this by adding a Boolean remove = false flag that gets passed through renderSubtreeIntoContainer() and into render() to bypass the removal behavior.

I played around with your demo for a bit. Here's where I left off: https://codesandbox.io/s/30q97oj8n1

marvinhagemeister commented 4 years ago

This is fixed in Preact X. Make sure to alias preact/compat instead of preact-compat.