Supported cloneVNode use-case?

[jods4]

What problem does this feature solve?

cloneVNode is public API for advanced scenarios.
I'm unsure what you're allowed to do with it or not and I encountered an interesting pitfall.

I have created a render function that manipulates its slot.
Basically:

• The slot is executed, returning an array of vnodes.
• Based on state, one child is modified by using cloneVNode(original, { class: 'added-stuff' })
• The render function then uses a modified array where the original vnode is replaced with its modified clone.

Based on this example, the added-stuff class is correctly rendered in DOM when added.
But when, based on state, I stop modifying the child and use the original vnode instead, the class is not removed.

I don't know if this is a supported scenario but I have traced the root cause of this.
It doesn't reproduce always, it depends on the original node and its patchFlags.
When cloning, cloneVNode changes the patchFlags to deoptimize dynamic props, given you might have changed any props. That's why the added stuff always appear.
But when returning back to the original, it has optimized patchFlags and only looks at the dynamic props, neglecting that it might lack the props added by the clone.

In my tests, when I hack the patchFlags of the original, everything works as expected.

What does the proposed API look like?

If what I'm doing is not completely crazy, then I suggest the following fix:

When comparing vnodes, instead of using the patchFlags from the new node only, combine it with the patchFlags from previous node.
In normal situations they'll be the same anyway.
In cases where you swap a node with a modified cloneVNode, that will use the less optimized patchFlags and make both directions (original -> clone, then clone -> original) work.

[underfin]

If use the patchFlags of previous node, the extra prop of you add { class: 'added-stuff' } will not patched to dom.

Current behavior is expected.Because you add extar props with cloneVNode and you want to this work on dom, the changed patchFlags is necessary.

If you want remove { class: 'added-stuff' } with dom, should use cloneVNode(original, { }) instead of the original.

[jods4]

@underfin I'm not suggesting that you use the patchFlags of the previous node, which would obviously create the same problem in reverse.

Rather, I suggest you combine the patchFlags from both nodes. If either one is deoptimized then all props should be compared.

If you want remove { class: 'added-stuff' } with dom, should use cloneVNode(original, { }) instead of the original.

That is not always practical.
Consider this example: the slot contains 100 vnodes. In state you have an index i that you want to modify (e.g. add a class).
If i changes, removing the class in the next update is not trivial as:

• i having changed, you'd need the previous value stored somewhere.
• Even if you stored the previous value, nodes might have been added/removed (think of v-for) so the node previously at i might now be at a different, unpredictable, index.

[underfin]

@jods4 Sorry for I'm not completely get your first said =.=

When comparing vnodes, instead of using the patchFlags from the new node only, combine it with the patchFlags from previous node.

This is necessary with use patchFlags from the new node. Example, the previous node is a element but the new node is a component.That will get error as use patchFlags from the previous node.

Consider this example: the slot contains 100 vnodes. In state you have an index i that you want to modify (e.g. add a class).
If i changes, removing the class in the next update is not trivial as:
i having changed, you'd need the previous value stored somewhere.
Even if you stored the previous value, nodes might have been added/removed (think of v-for) so the node previously at i might now be at a different, unpredictable, index.

I agree with you if use cloneVnode withv-for nodes and add extra props, that really maybe will get some unexpected result.

[jods4]

No worry.
Wouldn't it be ok if you specifically look at the FULL_PROPS bit from both?

[underfin]

Thanks for your explain :)

Are you mean add the code patchFlag = n1.patchFlag | n2.patchFlag with this code place?
https://github.com/vuejs/vue-next/blob/8c72b1febdd36e227ff27fa0f7ceaa434c47e1a8/packages/runtime-core/src/renderer.ts#L782

If this is your idea, I think it is useful under this situation.

[jods4]

Yes, something like that.
As you said, mixing other flags might be confusing, so maybe just the FULL_PROPS:

let { patchFlag, dynamicChildren, dirs } = n2;
// Consider both previous and new node because either one might have been replaced with cloneVNode or similar.
patchFlag |= (n1.patchFlag & PatchFlags.FULL_PROPS);