diff --git a/.changeset/clear-snapshot-prop-refs.md b/.changeset/clear-snapshot-prop-refs.md new file mode 100644 index 0000000000..9fe78ae8c6 --- /dev/null +++ b/.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. diff --git a/packages/react/runtime/__test__/snapshot/renderToOpcodes.test.jsx b/packages/react/runtime/__test__/snapshot/renderToOpcodes.test.jsx index 2942649006..5310b6f3d2 100644 --- a/packages/react/runtime/__test__/snapshot/renderToOpcodes.test.jsx +++ b/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(); }); }); diff --git a/packages/react/runtime/src/snapshot/snapshot/snapshot.ts b/packages/react/runtime/src/snapshot/snapshot/snapshot.ts index a682469bb9..2d4c7a4422 100644 --- a/packages/react/runtime/src/snapshot/snapshot/snapshot.ts +++ b/packages/react/runtime/src/snapshot/snapshot/snapshot.ts @@ -41,6 +41,65 @@ export const snapshotInstanceManager: { }, }; +function isRemovedSnapshot( + value: unknown, + removedSnapshots: WeakSet, +): boolean { + return (typeof value === 'object' || typeof value === 'function') && value !== null + && removedSnapshots.has(value); +} + +function clearRemovedSnapshotsFromArray( + values: unknown[], + removedSnapshots: WeakSet, + seen = new WeakSet(), +): 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 { + const removedSnapshots = new WeakSet(); + traverseSnapshotInstance(removedChild, snapshot => { + removedSnapshots.add(snapshot); + }); + return removedSnapshots; +} + +function clearTransientChildPropRefs(owner: SnapshotInstance, removedSnapshots: WeakSet): void { + const props = (owner as unknown as { props?: Record }).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> = {}; 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;