Skip to content

bug: RefList.syncComponentWithList potential stale-snapshot / mixed add-remove-reorder edge case #19

Description

@lucas-spin

Summary

RefList.syncComponentWithList(elements, container) takes a snapshot of the elements array and the container.children collection at call time, then reconciles DOM children against that snapshot. In edge cases where elements are simultaneously added, removed, and reordered in the same synchronous cycle, the snapshot may not produce the correct final DOM state.

Affected code

src/core/ref/RefList.tssyncComponentWithList:

static syncComponentWithList(elements: Element[], container: Element): void {
  // snapshot taken here
  const currentChildren = Array.from(container.children);
  // ... add missing, remove stale, reorder
}

Potential edge cases

1. Add + Remove in same cycle

If a subscriber calls syncComponentWithList with a list where element A has been removed and element D has been added (A→B→C becomes B→C→D), the function needs to handle A's removal and D's insertion in a single pass. If the container already has A as a child and the snapshot doesn't account for the pending removal, D may be inserted at the wrong position.

2. Reorder + Add simultaneously

If elements are reordered from [A, B, C] to [C, A, B] and D is inserted at index 1 ([C, D, A, B]), the single-pass reconciliation may place D incorrectly because insertBefore references are computed from the pre-reorder snapshot.

3. Mixed scenario

const list = refList([A, B, C]);
// In one update:
list.splice(0, 1); // remove A
list.push(D);      // add D
list.sort(...);    // reorder B, C, D
// → RefList subscriber fires once with [D, B, C] or similar
// syncComponentWithList([D, B, C], container) where container had [A, B, C]

The stale snapshot of container.children ([A, B, C]) may cause A to be removed correctly but the reorder step to fire with stale reference nodes.

Suggested test to expose the issue

it('handles simultaneous remove + add + reorder', () => {
  const container = document.createElement('div');
  const [a, b, c] = [1, 2, 3].map(() => document.createElement('span'));
  container.append(a, b, c);
  // Simultaneously: remove a, add d, reorder so result is [d, c, b]
  const d = document.createElement('span');
  RefList.syncComponentWithList([d, c, b], container);
  expect([...container.children]).toEqual([d, c, b]);
});

Recommendation

  1. Document the current behaviour explicitly: "syncComponentWithList is safe for single add, single remove, or pure reorder. Mixed add+remove+reorder in one call is undefined behaviour."
  2. Or: implement a stable-key reconciliation (similar to React's key-based diffing) that handles all cases correctly.

Roadmap alignment: Testing utilities and example tests / RefList stability.

cc @zico15

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions