fix(react): clear removed snapshot prop refs

.changeset/clear-snapshot-prop-refs.md

@@ -0,0 +1,5 @@
+---
+"@lynx-js/react": patch
+---
+
+Clear transient snapshot child props when removed snapshot subtrees are detached, preventing compiled `$*` child references from retaining deleted list holder or list item subtrees after removal.

packages/react/runtime/__test__/snapshot/renderToOpcodes.test.jsx

@@ -94,6 +94,59 @@ describe('renderToString', () => {
     `);
   });
 
+  it('should clear transient named child props when the child is removed', () => {
+    const child = createElement('text', null, 'Hello');
+    const vnode = createElement('view', { $0: child });
+
+    renderToString(vnode, new SnapshotInstance('root'));
+
+    expect(vnode.props.$0).toBe(child);
+    vnode.removeChild(child);
+
+    expect(vnode.props.$0).toBeUndefined();
+    expect('$0' in vnode.props).toBe(false);
+    expect(vnode.childNodes).toEqual([]);
+  });
+
+  it('should clear removed children from nested transient prop arrays', () => {
+    const child = createElement('text', null, 'Hello');
+    const vnode = createElement('view', { $0: [[child]] });
+
+    renderToString(vnode, new SnapshotInstance('root'));
+
+    expect(vnode.props.$0[0][0]).toBe(child);
+    vnode.removeChild(child);
+
+    expect(vnode.props.$0).toEqual([[undefined]]);
+    expect(vnode.childNodes).toEqual([]);
+  });
+
+  it('should clear all named child props', () => {
+    const child = createElement('text', null, 'Hello');
+    const vnode = createElement('view', { $0: child, $foo: child });
+
+    renderToString(vnode, new SnapshotInstance('root'));
+    vnode.removeChild(child);
+
+    expect(vnode.props.$0).toBeUndefined();
+    expect(vnode.props.$foo).toBeUndefined();
+  });
+
+  it('should tolerate cycles in transient prop arrays when clearing removed children', () => {
+    const child = createElement('text', null, 'Hello');
+    const vnode = createElement('view', { $0: child });
+
+    renderToString(vnode, new SnapshotInstance('root'));
+    const children = [child];
+    children.push(children);
+    vnode.props.$0 = children;
+    vnode.removeChild(child);
+
+    expect(vnode.props.$0[0]).toBeUndefined();
+    expect(vnode.props.$0[1]).toBe(children);
+    expect(vnode.childNodes).toEqual([]);
+  });
+
   it('should render Component depth 1', () => {
     function App() {
       const [a, setA] = useState(111);
@@ -1287,32 +1340,13 @@ describe('renderOpcodesInto', () => {
 
     const [vnodeA, vnodeB, vnodeC, vnodeC2, vnodeD] = scratch.__firstChild.props.$0;
 
-    expect(vnodeA).not.toHaveProperty('__elements');
-    expect(vnodeA).not.toHaveProperty('__element_root');
+    expect(vnodeA).toBeUndefined();
     expect(vnodeB).toHaveProperty('__elements');
     expect(vnodeB).toHaveProperty('__element_root');
     expect(vnodeC).toHaveProperty('__elements');
     expect(vnodeC).toHaveProperty('__element_root');
-    expect(vnodeD).not.toHaveProperty('__elements');
-    expect(vnodeD).not.toHaveProperty('__element_root');
-
-    {
-      const componentVNodeC = vnodeC2.props.$0;
-      expect(componentVNodeC.type).toBe(Fragment);
-      expect(componentVNodeC.props.children).toHaveLength(4);
-      // FIXME(hzy): there is still a cycle reference
-      expect(componentVNodeC.__c.__v).toBe(componentVNodeC);
-      componentVNodeC.props.children.forEach((vnode) => {
-        expect(vnode).not.toHaveProperty('__elements');
-        expect(vnode).not.toHaveProperty('__element_root');
-      });
-    }
-
-    expect(vnodeD.props.$0).toHaveLength(4);
-    vnodeD.props.$0.forEach((vnode) => {
-      expect(vnode).not.toHaveProperty('__elements');
-      expect(vnode).not.toHaveProperty('__element_root');
-    });
+    expect(vnodeC2).toBeUndefined();
+    expect(vnodeD).toBeUndefined();
   });
 });
 

packages/react/runtime/src/snapshot/snapshot/snapshot.ts

@@ -41,6 +41,65 @@ export const snapshotInstanceManager: {
   },
 };
 
+function isRemovedSnapshot(
+  value: unknown,
+  removedSnapshots: WeakSet<object>,
+): boolean {
+  return (typeof value === 'object' || typeof value === 'function') && value !== null
+    && removedSnapshots.has(value);
+}
+
+function clearRemovedSnapshotsFromArray(
+  values: unknown[],
+  removedSnapshots: WeakSet<object>,
+  seen = new WeakSet<object>(),
+): boolean {
+  if (seen.has(values)) {
+    return false;
+  }
+  seen.add(values);
+
+  let changed = false;
+  values.forEach((value, index) => {
+    if (isRemovedSnapshot(value, removedSnapshots)) {
+      values[index] = undefined;
+      changed = true;
+    } else if (Array.isArray(value) && clearRemovedSnapshotsFromArray(value, removedSnapshots, seen)) {
+      changed = true;
+    }
+  });
+  return changed;
+}
+
+function collectRemovedSnapshots(removedChild: SnapshotInstance): WeakSet<object> {
+  const removedSnapshots = new WeakSet<object>();
+  traverseSnapshotInstance(removedChild, snapshot => {
+    removedSnapshots.add(snapshot);
+  });
+  return removedSnapshots;
+}
+
+function clearTransientChildPropRefs(owner: SnapshotInstance, removedSnapshots: WeakSet<object>): void {
+  const props = (owner as unknown as { props?: Record<string, unknown> }).props;
+  if (!props) {
+    return;
+  }
+
+  for (const key of Object.keys(props)) {
+    // Named child props are transient staging refs. Once a child subtree is
+    // removed, they must not keep the old snapshots alive.
+    if (!key.startsWith('$')) {
+      continue;
+    }
+    const value = props[key];
+    if (isRemovedSnapshot(value, removedSnapshots)) {
+      delete props[key];
+    } else if (Array.isArray(value)) {
+      clearRemovedSnapshotsFromArray(value, removedSnapshots);
+    }
+  }
+}
+
 export let snapshotCreatorMap: Record<string, (uniqId: string) => string> = {};
 
 if (__DEV__ && __JS__) {
@@ -425,6 +484,8 @@ export class SnapshotInstance {
 
   removeChild(child: SnapshotInstance): void {
     const __snapshot_def = this.__snapshot_def;
+    const removedSnapshots = collectRemovedSnapshots(child);
+    clearTransientChildPropRefs(this, removedSnapshots);
     if (__snapshot_def.isListHolder) {
       if (__pendingListUpdates.values) {
         (__pendingListUpdates.values[this.__id] ??= new ListUpdateInfoRecording(
@@ -434,6 +495,7 @@ export class SnapshotInstance {
 
       this.__removeChild(child);
       traverseSnapshotInstance(child, v => {
+        clearTransientChildPropRefs(v, removedSnapshots);
         snapshotInstanceManager.values.delete(v.__id);
       });
       // mark this child as deleted
@@ -453,6 +515,7 @@ export class SnapshotInstance {
         snapshotDestroyList(v);
       }
 
+      clearTransientChildPropRefs(v, removedSnapshots);
       v.__parent = null;
       v.__previousSibling = null;
       v.__nextSibling = null;