Update render contexts while moving nodes

.changeset/selfish-maps-play.md

@@ -0,0 +1,5 @@
+---
+"@chialab/dna": patch
+---
+
+Reuse render context for referenced nodes.

.changeset/shy-yaks-tap.md

@@ -0,0 +1,5 @@
+---
+"@chialab/dna": patch
+---
+
+Fix runtime error while moving and replacing slot nodes in the same render cycle.

src/render.ts

@@ -39,12 +39,15 @@
     type: FunctionComponent | string | null;
     root?: Context;
     owner?: Context;
+    parent?: Context;
     children: Context[];
+    contexts: WeakMap<Node, Context>;
     properties?: KeyedProperties & TreeProperties & Record<string, unknown>;
     state?: HooksState;
     end?: Context;
     key?: unknown;
     keys?: Map<unknown, Context>;
+    refs?: Map<Node, Context>;
     _pos: number;
 };
 
@@ -70,6 +73,7 @@
     root,
     owner,
     children: [],
+    contexts: new WeakMap(),
     _pos: 0,
 });
 
@@ -283,8 +287,9 @@
  * Insert a node into the render tree.
  * @param parentContext The parent context.
  * @param childContext The child context.
+ * @param rootContext The root context.
  */
-const insertNode = (parentContext: Context, childContext: Context) => {
+const insertNode = (parentContext: Context, childContext: Context, rootContext: Context) => {
     const { node: parentNode, _pos: pos } = parentContext;
     const currentChildren = parentContext.children;
     if (currentChildren.includes(childContext)) {
@@ -293,12 +298,20 @@
         let currentContext: Context | null;
         while ((currentContext = currentChildren[pos]) && childContext !== currentContext) {
             parentNode.removeChild(currentContext.node);
+            rootContext.contexts.delete(currentContext.node);
             currentChildren.splice(pos, 1);
         }
     } else {
-        // we need to insert the new node into the tree
+        const currentChildContext = rootContext.contexts.get(childContext.node);
+        if (currentChildContext?.parent && currentChildContext.parent !== parentContext) {
+            const { node: currentParentNode, children: currentParentChildren } = currentChildContext.parent;
+            currentParentNode.removeChild(childContext.node);
+            currentParentChildren.splice(currentParentChildren.indexOf(currentChildContext), 1);
+        }
         parentNode.insertBefore(childContext.node, currentChildren[pos]?.node);
         currentChildren.splice(pos, 0, childContext);
+        childContext.parent = parentContext;
+        rootContext.contexts.set(childContext.node, childContext);
     }
     parentContext._pos++;
 };
@@ -310,6 +323,7 @@
  * @param template The template to render in Virtual DOM format.
  * @param namespace The current namespace uri of the render.
  * @param keys The current keys map of the render.
+ * @param refs The current refs map of the render.
  * @param realm The realm to use for the render.
  * @param fragment The fragment context to update.
  */
@@ -319,6 +333,7 @@
     template: Template,
     namespace: string,
     keys: Map<unknown, Context> | undefined,
+    refs: Map<Node, Context> | undefined,
     realm?: Realm,
     fragment: Context = context
 ) => {
@@ -332,13 +347,13 @@
             return;
         }
         if (len === 1) {
-            renderTemplate(context, rootContext, template[0], namespace, keys, realm, fragment);
+            renderTemplate(context, rootContext, template[0], namespace, keys, refs, realm, fragment);
             return;
         }
 
         // call the render function for each child
         for (let i = 0; i < len; i++) {
-            renderTemplate(context, rootContext, template[i], namespace, keys, realm, fragment);
+            renderTemplate(context, rootContext, template[i], namespace, keys, refs, realm, fragment);
         }
         return;
     }
@@ -348,7 +363,7 @@
 
     if (isVObject(template)) {
         if (isVFragment(template)) {
-            renderTemplate(context, rootContext, template.children, namespace, keys, realm, fragment);
+            renderTemplate(context, rootContext, template.children, namespace, keys, refs, realm, fragment);
             return;
         }
 
@@ -373,6 +388,7 @@
                 rootNode || document.createComment(Function.name),
                 namespace,
                 keys,
+                refs,
                 realm,
                 fragment
             );
@@ -438,6 +454,7 @@
                 ),
                 namespace,
                 keys,
+                refs,
                 realm
             );
 
@@ -454,9 +471,9 @@
             const name = properties?.name;
             const slotted = realm.childNodesBySlot(name);
             if (slotted.length) {
-                renderTemplate(context, rootContext, slotted, namespace, keys, realm, fragment);
+                renderTemplate(context, rootContext, slotted, namespace, keys, refs, realm, fragment);
             } else if (children) {
-                renderTemplate(context, rootContext, children, namespace, keys, realm, fragment);
+                renderTemplate(context, rootContext, children, namespace, keys, refs, realm, fragment);
             }
             return;
         }
@@ -489,9 +506,8 @@
         if (!templateContext) {
             if (isVNode(template)) {
                 const node = template.type;
-                templateContext =
-                    currentChildren.find((child) => child.node === node) ||
-                    createContext(ContextKind.REF, null, template.type, rootContext);
+                templateContext = refs?.get(node) || createContext(ContextKind.REF, null, template.type, rootContext);
+                fragment.refs = (fragment.refs || new Map()).set(node, templateContext);
             } else {
                 const constructor = customElements?.get(properties?.is ?? template.type);
                 templateContext = createContext(
@@ -552,7 +568,7 @@
             (node as ComponentInstance).collectUpdatesEnd();
         }
 
-        insertNode(context, templateContext);
+        insertNode(context, templateContext, rootContext);
 
         if ((templateContext && children && children.length) || templateContext.root === rootContext) {
             internalRender(templateContext, children, realm, rootContext, namespaceURI, undefined);
@@ -563,7 +579,8 @@
         insertNode(
             context,
             currentChildren.find((child) => child.node === template) ||
-                createContext(ContextKind.REF, null, template, rootContext)
+                createContext(ContextKind.REF, null, template, rootContext),
+            rootContext
         );
         return;
     }
@@ -583,7 +600,7 @@
             currentContext.type = template;
             currentContext.node.nodeValue = template as string;
         }
-        insertNode(context, currentContext);
+        insertNode(context, currentContext, rootContext);
         return;
     }
 
@@ -596,7 +613,8 @@
             document.createTextNode(template as string),
             rootContext,
             rootContext
-        )
+        ),
+        rootContext
     );
 };
 
@@ -624,18 +642,21 @@
 
     let endContext: Context | undefined;
     let currentKeys: Map<unknown, Context> | undefined;
+    let currentRefs: Map<Node, Context> | undefined;
     if (fragment) {
         context._pos = context.children.indexOf(fragment);
         endContext = fragment.end as Context;
         currentKeys = fragment.keys;
+        currentRefs = fragment.refs;
         delete fragment.keys;
     } else {
         context._pos = 0;
         currentKeys = context.keys;
+        currentRefs = context.refs;
         delete context.keys;
     }
 
-    renderTemplate(context, rootContext, template, namespace, currentKeys, realm, fragment);
+    renderTemplate(context, rootContext, template, namespace, currentKeys, currentRefs, realm, fragment);
 
     // all children of the root have been handled,
     // we can start to cleanup the tree
@@ -649,11 +670,7 @@
     }
 
     while (currentIndex <= --end) {
-        const [child] = contextChildren.splice(end, 1);
-        const parentNode = child.node.parentNode;
-        if (parentNode === context.node || (realm?.open && parentNode === realm.node && realm.root === context.node)) {
-            context.node.removeChild(child.node);
-        }
+        context.node.removeChild(contextChildren.splice(end, 1)[0].node);
     }
 
     return contextChildren;

test/render.spec.js

@@ -696,6 +696,56 @@ describe.runIf(typeof window !== 'undefined')(
 
                 expect(child.textContent).toBe('Hello');
             });
+
+            it('should resuse refs context', () => {
+                const name = getComponentName();
+                DNA.define(
+                    name,
+                    class TestElement extends DNA.Component {
+                        static get properties() {
+                            return {
+                                showList: {
+                                    type: Boolean,
+                                    defaultValue: true,
+                                },
+                                items: {
+                                    type: Array,
+                                },
+                            };
+                        }
+
+                        list = this.ownerDocument.createElement('ul');
+
+                        render() {
+                            if (this.showList) {
+                                return DNA.h('ul', { ref: this.list }, [
+                                    ...this.items.map((item) => DNA.h('li', null, item)),
+                                ]);
+                            }
+                        }
+                    }
+                );
+
+                const element = document.createElement(name);
+                element.items = ['one', 'two', 'three'];
+                wrapper.appendChild(element);
+
+                expect(element.list.childNodes).toHaveLength(3);
+                expect(element.list.childNodes[0].textContent).toBe('one');
+                expect(element.list.childNodes[1].textContent).toBe('two');
+                expect(element.list.childNodes[2].textContent).toBe('three');
+
+                element.showList = false;
+                element.items = ['four', 'five'];
+
+                expect(element.list.isConnected).toBe(false);
+
+                element.showList = true;
+
+                expect(element.list.childNodes).toHaveLength(2);
+                expect(element.list.childNodes[0].textContent).toBe('four');
+                expect(element.list.childNodes[1].textContent).toBe('five');
+            });
         });
 
         describe('hooks', () => {

test/template.spec.js

@@ -490,6 +490,56 @@ describe.runIf(typeof window !== 'undefined')(
             }
         });
 
+        describe('slot moved and replaced', () => {
+            const TEMPLATES = {
+                JSX(switchProp) {
+                    return [
+                        DNA.h('div', { class: 'parent-1' }, switchProp ? DNA.h('slot') : 'Empty'),
+                        DNA.h('div', { class: 'parent-2' }, !switchProp ? DNA.h('slot') : 'Empty'),
+                    ];
+                },
+                HTML(switchProp) {
+                    return DNA.html`
+                        <div class="parent-1">${switchProp ? DNA.html`<slot />` : 'Empty'}</div>
+                        <div class="parent-2">${!switchProp ? DNA.html`<slot />` : 'Empty'}</div>
+                    `;
+                },
+            };
+
+            for (const type in TEMPLATES) {
+                it(type, () => {
+                    const name = getComponentName();
+                    DNA.define(
+                        name,
+                        class TestElement extends DNA.Component {
+                            static get properties() {
+                                return {
+                                    switch: {
+                                        type: Boolean,
+                                    },
+                                };
+                            }
+
+                            render() {
+                                return TEMPLATES[type](this.switch);
+                            }
+                        }
+                    );
+
+                    const element = document.createElement(name);
+                    element.appendChild(document.createTextNode('Hello'));
+
+                    expect(element.querySelector('.parent-1').textContent).toBe('Empty');
+                    expect(element.querySelector('.parent-2').textContent).toBe('Hello');
+
+                    element.switch = true;
+
+                    expect(element.querySelector('.parent-1').textContent).toBe('Hello');
+                    expect(element.querySelector('.parent-2').textContent).toBe('Empty');
+                });
+            }
+        });
+
         describe('events', () => {
             describe('should add an event listener', () => {
                 const TEMPLATES = {