Preact: Diff behaviour not as expected - preact unmounting then remounting the same DOM nodes even when there is a key

Created on 31 Mar 2018  Â·  17Comments  Â·  Source: preactjs/preact

I have found that preact is unecessarily tearing down and rebuilding DOM nodes even when a key is used (example codepen)

In this case even though the key is supplied almost all of the DOM nodes are being removed and then added again even though only three are changed

EDIT
TO be clear it is expected that the components are being rerendered, but the issue is that all of the DOM nodes (not virtual dom) are being removed and then re-added again

(before changing the document):

screen shot 2018-03-31 at 9 31 36 pm

(after changing the document):

screen shot 2018-03-31 at 9 31 38 pm

After debugging I eventually found that each time the new document was selected the span elements (in the real DOM) inside the content were being removed and then added again, and the weird thing is that the spans that are being removed are exactly the same as the spans that are being added (as in they were the same instance)

Expected Behaviour
Only a few DOM nodes will be changed (as keys are supplied and most of the vdom remains the same)

Actual Behaviour
All of the DOM nodes are being removed and then re-added again

bug help wanted

Most helpful comment

That's great news! Is there a timeline for that release?

All 17 comments

Just as a side note so you dont get confused I am providing the key attribute as well as the data-key attribute for debugging

It’s because you are intentionally switching
component(oldDocument<->newDocument). A component change will definitely
tear down the old one and setup the new one.

On Sat, Mar 31, 2018 at 06:45 PandaSandStudio notifications@github.com
wrote:

Just as a side note so you dont get confused I am providing the key
attribute as well as the data-key attribute for debugging

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/developit/preact/issues/1052#issuecomment-377684097,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AK3Wt70H7Ql086-EOZUXxGpdPWUgD906ks5tj14zgaJpZM4TCa4G
.

I'm sorry if I misunderstand, but I'm not switching out the component I'm switching out the property. The issue isn't that the Document component is being re-rendered it is that all of the DOM nodes in the document (not the vdom) are being removed and then re-added again even though I am providing a key and the vdom is barely changing.

For clarity I edited the original question

Yes I see your point. My previous comment is wrong. Will investigate this
later

On Sat, Mar 31, 2018 at 20:10 PandaSandStudio notifications@github.com
wrote:

For clarity I edited the original question

—
You are receiving this because you commented.

Reply to this email directly, view it on GitHub
https://github.com/developit/preact/issues/1052#issuecomment-377732646,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AK3Wt8jONuT35_F9nG7_L68cNG6Zch4wks5tkBrigaJpZM4TCa4G
.

Ok thanks. Just for reference I created another codepen using React (https://codepen.io/HuntingLamas/pen/mxLWBV?editors=0110)

In the React case only a few DOM nodes are changed
In the Preact case most of the DOM nodes are stripped then re-mounted

BTW I think Preact is a great library and thanks for all of your work!

NOTE I have created a new pen and changed the text of the elements to help with debugging

I have found that when preact is diffing, if the original html is like this:

<span class="bold" data-key="08c2ea62-5974-4ee7-a678-232e8cf46b57">111</span>
<span class="" data-key="8b8fceeb-ffbc-4a0a-a34e-d803508cab4e">2.0</span>
<span class="bold" data-key="2010573b-5b75-4b9b-84d2-a2d9b696f061">2.1</span>
<span class="" data-key="33576a1d-0514-4d10-a5a7-f8b4b8c83e5e">2.2</span>
<span class="bold" data-key="76e151f3-e96b-4ec5-b134-dcb991c84c89">333</span>
<span class="" data-key="2df9566c-b3e5-4b4f-bede-0656902cb6ed">444</span>
<span class="bold" data-key="639c9cee-2a6a-4988-ade6-82751851ab98">555</span>
<span class="" data-key="46414427-90ca-4444-8090-ace7821c003a">666</span>
<span class="bold" data-key="fcee30f3-ef0c-4ff0-a5c3-0ad585e74c96">777</span>
<span class="" data-key="82f6266f-9447-481c-adcc-3f7695c0fe33">888</span>

and the new HTML is this:

<span class="bold" data-key="08c2ea62-5974-4ee7-a678-232e8cf46b57">111</span>
<span class="" data-key="8b8fceeb-ffbc-4a0a-a34e-d803508cab4e">2.02.12.2</span>
<span class="bold" data-key="76e151f3-e96b-4ec5-b134-dcb991c84c89">333</span>
<span class="" data-key="2df9566c-b3e5-4b4f-bede-0656902cb6ed">444</span>
<span class="bold" data-key="639c9cee-2a6a-4988-ade6-82751851ab98">555</span>
<span class="" data-key="46414427-90ca-4444-8090-ace7821c003a">666</span>
<span class="bold" data-key="fcee30f3-ef0c-4ff0-a5c3-0ad585e74c96">777</span>
<span class="" data-key="82f6266f-9447-481c-adcc-3f7695c0fe33">888</span>

Then preact will carry out the following operations (in the order provided)

screenshot 2018-04-01 at 11 33 48

Which I believe means that for some reason preact moves the spans which are after

<span class="" data-key="33576a1d-0514-4d10-a5a7-f8b4b8c83e5e">2.2</span>

(aka the ones with text 333,444,555,etc.) to before

<span class="bold" data-key="2010573b-5b75-4b9b-84d2-a2d9b696f061">2.1</span>

and then deletes

<span class="bold" data-key="2010573b-5b75-4b9b-84d2-a2d9b696f061">2.1</span>

and

<span class="" data-key="33576a1d-0514-4d10-a5a7-f8b4b8c83e5e">2.2</span>

and then sets the text of

<span class="" data-key="8b8fceeb-ffbc-4a0a-a34e-d803508cab4e">2.0</span>

to 2.02.12.2

Which seems very unecessary (note that this doesnt happen in react - react just removes 2.1 and 2.2 and sets the text of 2.0 to 2.02.12.2)

Here is the codepen for reference (when you switch from First to Second by clicking you should see the operations printed to the console)

This is indeed a bug. Preact cannot correctly handle removing multiple children from non-end position. If you remove just one child from any position, or multiple children from the end of the list, they both work fine.

I think there is already one ticket tracking this and I have not found it yet

IIRC that bug only affects unkeyed children, right?

@yaodingyd this is fixed in ceviche for arbitrary removals AFAIK

@developit I think this is different from the unkeyed children bug.

This one is because this line:
https://github.com/developit/preact/blob/1369ab358a735a67d33c1287e4c15c8a1c85ef58/src/vdom/diff.js#L250
So if there are multiple children to be removed, nextSibling would not work and the above line would not be truthy. So instead of just removing these children, we first insert their following children before them, then remove them at the end.

The unkeyed children bug is plucking the same node type existing children results in unnecessary diffing work.

Right?

@yaodingyd you're right, I was conflating the two. I wonder if we can check against the keyed child map within that conditional...

@developit I'm thinking if we could have another map of keyed vnodes and when originalchild has a key, check against this map. If returns undefined then remove f

Has there been any movement on this? I'm encountering the same bug (originally referenced in #1069). Is there anything blocking? I'm not super familiar with internal workings of Preact, but if there's something I can do to help in my spare time I'd be glad to! :slightly_smiling_face:

@lewischa this is difficult to fix in the currently version of Preact, but it has been fixed in the next major version.

That's great news! Is there a timeline for that release?

I'm experiencing the same problem. I have a CSS entry animation on my child nodes and reordering them triggers the animation again :(

Good news: Just tried the snippet in the original post with the code in master and I can't reproduce the issue anymore :tada: :+1:

Was this page helpful?
0 / 5 - 0 ratings

Related issues

marcosguti picture marcosguti  Â·  3Comments

k15a picture k15a  Â·  3Comments

matuscongrady picture matuscongrady  Â·  3Comments

mizchi picture mizchi  Â·  3Comments

kay-is picture kay-is  Â·  3Comments