Skip to content

fix: prevent stale observerMap array notifications after replacement in fast-html #7579

Description

@janechu

🐛 Bug Report

ObserverMap-managed arrays can keep stale array observer subscriptions after an array branch is replaced by deepMerge. If external code retains a reference to the old root array, or to a nested array inside the replaced branch, later mutations to that stale array can still notify the owning FAST element's root property even though the current data no longer contains that array.

This is a follow-up hardening issue for #7559.

💻 Repro or Code Sample

Conceptual repro:

const oldOrders = element.user.orders;
const oldItems = oldOrders[0].items;

deepMerge(element.user, {
    orders: [
        {
            id: 103,
            items: [{ name: "Replacement", metrics: { views: 1 } }],
        },
    ],
});

oldOrders.push({ id: 104, items: [] });
oldItems.push({ name: "Stale", metrics: { views: 2 } });

The stale oldOrders and oldItems references should no longer be able to drive notifications or renders for the current element.user tree.

Also cover a shared-array case:

const shared = [{ name: "shared" }];
ownerA.data.items = shared;
ownerB.data.items = shared;

deepMerge(ownerA.data, { items: [{ name: "replacement" }] });
shared.push({ name: "still owned by B" });

Mutating shared should still notify ownerB, but not ownerA.

🤔 Expected Behavior

Array observer registrations should be scoped to the owner/root property/schema context that installed them.

When an observerMap-managed root or nested array is no longer reachable from a target's current root data, mutations to that stale array should not call Observable.notify(target, rootProperty) for that target. If the same array instance is still reachable from another owner, that other owner should continue to receive notifications.

😯 Current Behavior

The current implementation tracks observed arrays with a WeakSet, which only models whether an array has been observed at least once. It does not distinguish between multiple owner/root-property/schema contexts for shared arrays, and stale subscriptions on replaced arrays may still be live.

💁 Possible Solution

Replace observedArraysMap: WeakSet<object> with per-array registrations, for example WeakMap<object, ArrayObserverRegistration[]>.

Each registration should store the subscriber object passed to Observable.getNotifier(data).subscribe(...). subscribe() returns void; it does not return a subscriber/disposable.

Track enough context to dedupe only the same observation context while allowing shared arrays to be observed by multiple owners: target, rootProperty, item schema, rootSchema, and subscriber.

Guard array subscriber work by checking that the changed array is still reachable from target[rootProperty] for that registration before proxying added items or notifying the root property. A root-level check such as target[rootProperty] === subject is not sufficient because nested arrays are valid subjects too.

If explicit teardown is added, apply it consistently from both replacement paths: the setter created by defineObservableProperty and the assignProxy set trap. Teardown should remove only registrations for the relevant owner context and must not unsubscribe registrations still needed by another owner or by a shared branch that remains reachable.

Suggested acceptance coverage:

  • A Playwright test that holds a reference to the original root array, calls deepMerge to replace it, mutates the stale reference, and asserts that no parent notification/render is triggered.
  • A Playwright test that holds a reference to a nested array inside the replaced branch, mutates it after replacement, and asserts that no parent notification/render is triggered.
  • A Playwright test where the same array instance is observed by two owners; after one owner replaces its property, mutations through the shared array still notify the owner that continues to reference it.
  • Coverage for both deepMerge replacement and direct property/proxy assignment replacement paths.

🔦 Context

PR #7559 intentionally changed observerMap deepMerge behavior to replace arrays instead of mutating observed arrays in place, avoiding synchronous reentrant array notification work. This follow-up closes remaining edge cases around stale retained references, nested arrays, and shared array instances.

Searched existing issues for overlapping observerMap/deepMerge/stale array subscription reports and did not find a duplicate.

🌍 Your Environment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions