fix(runtime): append newChild if parent node doesn't match with patched node

[itsjustaplant]

What is the current behavior?

GitHub Issue Number: #6140

What is the new behavior?

If patched node (this) is not the same node of currentChild, it uses appendChild to insert the newChild to patched node

Documentation

Does this introduce a breaking change?

• Yes
• No

Testing

Other information

[christian-bromann]

@itsjustaplant thanks for raising the PR, mind creating a test to ensure we don't regress on this? I recommend to create a WebdriverIO test as it provides the most flexibility creating the test. Let me know if you have any questions.

[itsjustaplant]

Hi @christian-bromann , I am unable to reproduce this issue in either a plain Stencil app or WebdriverIO tests. I believe that this only happens in React apps. PS: I replaced the unpatched insertBefore with appendChild as a fallback method as i believe that appendChild is safer.

[christian-bromann]

@itsjustaplant mhm 🤔 it would be good if we could validate this fix somehow, otherwise it may become difficult at some point to understand why we've put this in place or whether it still is relevant to Stencil.

[itsjustaplant]

Hi @christian-bromann , I have a reproduction repository here. If you run the my-app, you will see the error in couple of seconds due to mocked fetch. As it only happens in react, i don't know what more could i add as reproduction but this commit somehow introduced the bug on stencil side (it was previously on react-dom).

const parentNode = (currentChild as d.PatchedSlotNode)?.__parentNode;
if (parentNode && !this.isSameNode(parentNode)) {
    return this.appendChild(newChild);
}

You can consider this as a fallback that is triggered when parentNodes doesn't match.
If parentNodes are not same we simply call appendChild as it will handle the placement of the slotted node.
And if parentNodes match, the original return (this as d.RenderNode).__insertBefore(newChild, currentChild); will be called.

[yigityuce]

@itsjustaplant mhm 🤔 it would be good if we could validate this fix somehow, otherwise it may become difficult at some point to understand why we've put this in place or whether it still is relevant to Stencil.

hi @christian-bromann hope you are well since from last touchpoint between us. Alperen (@itsjustaplant) is one of our team members and as he said unfortunately we are having this issue only in react. Probably react does bulk update on DOM after stencil does update on the DOM, and, while react was trying to use insertBefore, the parentNode is not in the previous place where react stores in its vdom. (my strong guess is react does these kind of bulk operations in nextTick, and in the nextTick the actual DOM is already changed by Stencil)

So, unfortunately WDIO test is not able to be providable. However, I understand your concern and agree with you. Maybe we can put some more detail to the comment section in the code that describes the issue better. Would it work for you?