Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "Ignore stale observer map array notifications after replacement.",
"packageName": "@microsoft/fast-element",
"email": "pradeepramolaa@gmail.com",
"dependentChangeType": "none"
}
235 changes: 198 additions & 37 deletions packages/fast-element/src/declarative/observer-map-utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,26 @@ interface ObservedTargetsAndProperties {
rootProperty: string;
}

interface ArrayObserverRegistration {
target: any;
rootProperty: string;
schema: JSONSchema | JSONSchemaDefinition;
rootSchema: JSONSchema;
reachableArray: any;
subscriber: {
handleChange(subject: any, args: any[]): void;
};
}

/**
* A map of proxied objects.
*/
const objectTargetsMap = new WeakMap<object, ObservedTargetsAndProperties[]>();

/**
* A map of arrays being observed.
* A map of arrays to their owner-specific observer registrations.
*/
const observedArraysMap = new WeakSet<object>();
const observedArraysMap = new WeakMap<object, ArrayObserverRegistration[]>();

type DataType = "array" | "object" | "primitive";

Expand All @@ -35,6 +46,10 @@ function getDataType(data: any): DataType {
return "primitive";
}

function isObjectLike(value: any): value is object {
return typeof value === "object" && value !== null;
}

/**
* Get properties from an anyOf array
* @param anyOf - The anyOf array in a JSON schema
Expand Down Expand Up @@ -104,7 +119,6 @@ function isSchemaExcluded(schema: any): boolean {
return schema?.$observe === false && !hasObservedSchemaDescendant(schema);
}


function defineObservableProperty(
targetObject: any,
key: string,
Expand Down Expand Up @@ -156,6 +170,181 @@ function findArrayItemDef(schema: JSONSchema | JSONSchemaDefinition): string | n
return findDef(schema) ?? (items === undefined ? null : findDef(items));
}

function isReachableFromRoot(root: any, value: any): boolean {
if (root === value) {
return true;
}

if (!isObjectLike(root) || !isObjectLike(value)) {
return false;
}

const seen = new WeakSet<object>();
const stack = [root];

while (stack.length > 0) {
const current = stack.pop();

if (current === value) {
return true;
}

if (!isObjectLike(current) || seen.has(current)) {
continue;
}

seen.add(current);

for (const key of Object.keys(current)) {
if (key === "$fastController") {
continue;
}

stack.push(current[key]);
}
}

return false;
}

function isArrayRegistrationReachable(
registration: ArrayObserverRegistration,
subject: any,
): boolean {
const root = registration.target?.[registration.rootProperty];

return (
isReachableFromRoot(root, subject) ||
isReachableFromRoot(root, registration.reachableArray)
);
}

function isSameArrayObserverContext(
registration: ArrayObserverRegistration,
target: any,
rootProperty: string,
schema: JSONSchema | JSONSchemaDefinition,
rootSchema: JSONSchema,
): boolean {
return (
registration.target === target &&
registration.rootProperty === rootProperty &&
registration.schema === schema &&
registration.rootSchema === rootSchema
);
}

function handleArrayChange(
registration: ArrayObserverRegistration,
subject: any,
args: any[],
) {
if (!isArrayRegistrationReachable(registration, subject)) {
removeArrayRegistration(subject, registration);
return;
}

const schemaProperties = getSchemaProperties(registration.schema);

args.forEach((arg: any) => {
if (arg.addedCount > 0) {
if (schemaProperties) {
for (let i = arg.addedCount - 1; i >= 0; i--) {
const item = subject[arg.index + i];
const originalItem = Object.assign({}, item);

assignProxyToItemsInArray(
item,
originalItem,
registration.schema,
registration.rootSchema,
registration.target,
registration.rootProperty,
);

Object.assign(item, originalItem);
}
}

Observable.notify(registration.target, registration.rootProperty);
}
});
}

function removeArrayRegistration(
data: any[],
registration: ArrayObserverRegistration,
): void {
const registrations = observedArraysMap.get(data);

if (!registrations) {
return;
}

const registrationIndex = registrations.indexOf(registration);

if (registrationIndex === -1) {
return;
}

registrations.splice(registrationIndex, 1);

if (registrations.length === 0) {
observedArraysMap.delete(data);
}

Promise.resolve().then(() => {
Observable.getNotifier(data).unsubscribe(registration.subscriber);
});
}

function observeArray(
data: any[],
reachableArray: any,
schema: JSONSchema | JSONSchemaDefinition,
rootSchema: JSONSchema,
target: any,
rootProperty: string,
): void {
let registrations = observedArraysMap.get(data);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

registrations are never cleaned up, a subscriber may have an old notifier attached, can you ensure these get cleaned up?


if (!registrations) {
registrations = [];
observedArraysMap.set(data, registrations);
}

const existingRegistration = registrations.find(registration =>
isSameArrayObserverContext(
registration,
target,
rootProperty,
schema,
rootSchema,
),
);

if (existingRegistration) {
existingRegistration.reachableArray = reachableArray;
return;
}

const registration: ArrayObserverRegistration = {
target,
rootProperty,
schema,
rootSchema,
reachableArray,
subscriber: {
handleChange(subject, args) {
handleArrayChange(registration, subject, args);
},
},
};

registrations.push(registration);
Observable.getNotifier(data).subscribe(registration.subscriber);
}

/**
* Assigns Observable properties to items in an array and sets up change notifications
* @param proxiedData - The array data to make observable
Expand Down Expand Up @@ -193,40 +382,8 @@ function assignObservablesToArray(
})
: proxiedData;

if (!observedArraysMap.has(data)) {
observedArraysMap.add(data);

Observable.getNotifier(data).subscribe({
handleChange(subject, args) {
args.forEach((arg: any) => {
if (arg.addedCount > 0) {
if (schemaProperties) {
for (let i = arg.addedCount - 1; i >= 0; i--) {
const item = subject[arg.index + i];
const originalItem = Object.assign({}, item);

assignProxyToItemsInArray(
item,
originalItem,
schema,
rootSchema,
target,
rootProperty,
);

Object.assign(item, originalItem);
}
}

// Notify observers of the target object's root property
Observable.notify(target, rootProperty);
}
});
},
});
}

if (schemaProperties !== null) {
observeArray(data, data, schema, rootSchema, target, rootProperty);
return data;
}

Expand All @@ -236,7 +393,7 @@ function assignObservablesToArray(
// their items are individually proxied, and FAST's own push/splice/etc.
// already carry splice records β€” double-wrapping would produce duplicate
// splice notifications.
return new Proxy(data, {
const primitiveArrayProxy = new Proxy(data, {
set: (arr: any, prop: string | symbol, value: any) => {
const idx = typeof prop === "string" ? Number(prop) : NaN;

Expand All @@ -251,6 +408,10 @@ function assignObservablesToArray(
return true;
},
});

observeArray(data, primitiveArrayProxy, schema, rootSchema, target, rootProperty);

return primitiveArrayProxy;
}

/**
Expand Down
Loading