#2168: Added unit tests to check proper component unmounting#2195
Conversation
| } | ||
| } | ||
|
|
||
| // The only difference to the previous test is the extra div here |
There was a problem hiding this comment.
Yes but this <div> is not part of your vnode tree since you start underneath and then target it with newScratch.firstElementChild did this work with Preact 8? This isn't supported in Preact X atleast since this falls outside your render root.
<scratch>
<div> <-- you start replacing node here
<div id="child"> <-- your root starts here
</scratch>
^ this is intended
|
I don't mind merging in the first test, the second has been discussed on your issue right? Do you think we could do more to help out in this case? |
|
Yes, the issue has been discussed and there is a reasonable workaround for now. Keeping the first test would still be great like you said. Thank's for the support! |
JoviDeCroock
left a comment
There was a problem hiding this comment.
Thanks for this, more tests prevents us from accidentally shipping regressions!
|
🚀 This PR has been merged! Once a new release is created, any changes will become available on npm. Until then, you can load and install it directly from the Pika CDN: |
…reactjs#2195) * preactjs#2168: Added unit tests to check proper component unmounting * preactjs#2168: Remove test onlies * remove the second test (invalid) Co-authored-by: Jovi De Croock <decroockjovi@gmail.com>
I added two tests that show the bug: #2168