fix: defend against for/in use on arrays (FE2) (#5662)

* fix: defend against for/in use on arrays

* Change files

* test: added some basic tests for array observers

Co-authored-by: EisenbergEffect <[email protected]>

change/@microsoft-fast-element-8e5ad334-ac12-466b-8440-415abd011eb0.json

@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "fix: defend against for/in use on arrays",
+  "packageName": "@microsoft/fast-element",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}

packages/web-components/fast-element/src/observation/array-observer.spec.ts

@@ -0,0 +1,56 @@
+import { expect } from "chai";
+import { Observable } from "./observable";
+import { enableArrayObservation } from "./array-observer";
+import { SubscriberSet } from "./notifier";
+
+describe("The ArrayObserver", () => {
+    it("can be retrieved through Observable.getNotifier()", () => {
+        enableArrayObservation();
+        const array = [];
+        const notifier = Observable.getNotifier(array);
+        expect(notifier).to.be.instanceOf(SubscriberSet);
+    });
+
+    it("is the same instance for multiple calls to Observable.getNotifier() on the same array", () => {
+        enableArrayObservation();
+        const array = [];
+        const notifier = Observable.getNotifier(array);
+        const notifier2 = Observable.getNotifier(array);
+        expect(notifier).to.equal(notifier2);
+    });
+
+    it("is different for different arrays", () => {
+        enableArrayObservation();
+        const notifier = Observable.getNotifier([]);
+        const notifier2 = Observable.getNotifier([]);
+        expect(notifier).to.not.equal(notifier2);
+    });
+
+    it("doesn't affect for/in loops on arrays when enabled", () => {
+        enableArrayObservation();
+
+        const array = [1, 2, 3];
+        const keys: string[] = [];
+
+        for (const key in array) {
+            keys.push(key);
+        }
+
+        expect(keys).eql(["0", "1", "2"]);
+    });
+
+    it("doesn't affect for/in loops on arrays when the array is observed", () => {
+        enableArrayObservation();
+
+        const array = [1, 2, 3];
+        const keys: string[] = [];
+        const notifier = Observable.getNotifier(array);
+
+        for (const key in array) {
+            keys.push(key);
+        }
+
+        expect(notifier).to.be.instanceOf(SubscriberSet);
+        expect(keys).eql(["0", "1", "2"])
+    });
+});

packages/web-components/fast-element/src/observation/array-observer.ts

@@ -4,6 +4,13 @@ import { SubscriberSet } from "./notifier.js";
 import type { Notifier } from "./notifier.js";
 import { Observable } from "./observable.js";
 
+function setNonEnumerable(target: any, property: string, value: any) {
+    Reflect.defineProperty(target, property, {
+        value,
+        enumerable: false,
+    });
+}
+
 class ArrayObserver extends SubscriberSet {
     private oldCollection: any[] | undefined = void 0;
     private splices: Splice[] | undefined = void 0;
@@ -12,7 +19,7 @@ class ArrayObserver extends SubscriberSet {
 
     constructor(subject: any[]) {
         super(subject);
-        (subject as any).$fastController = this;
+        setNonEnumerable(subject, "$fastController", this);
     }
 
     public addSplice(splice: Splice): void {
@@ -73,15 +80,13 @@ export function enableArrayObservation(): void {
     enabled = true;
 
     Observable.setArrayObserverFactory(
-        (collection: any[]): Notifier => {
-            return new ArrayObserver(collection);
-        }
+        (collection: any[]): Notifier => new ArrayObserver(collection)
     );
 
     const proto = Array.prototype;
 
     if (!(proto as any).$fastPatch) {
-        (proto as any).$fastPatch = true;
+        setNonEnumerable(proto, "$fastPatch", 1);
 
         const pop = proto.pop;
         const push = proto.push;

packages/web-components/fast-element/src/observation/observable.spec.ts

@@ -76,19 +76,11 @@ describe("The Observable", () => {
             expect(notifier).to.equal(notifier2);
         });
 
-        it("can get a notifier for an array", () => {
-            enableArrayObservation();
-            const array = [];
-            const notifier = Observable.getNotifier(array);
-            expect(notifier).to.be.instanceOf(SubscriberSet);
-        });
+        it("gets different notifiers for different objects", () => {
+            const notifier = Observable.getNotifier(new Model());
+            const notifier2 = Observable.getNotifier(new Model());
 
-        it("gets the same notifier for the same array", () => {
-            enableArrayObservation();
-            const array = [];
-            const notifier = Observable.getNotifier(array);
-            const notifier2 = Observable.getNotifier(array);
-            expect(notifier).to.equal(notifier2);
+            expect(notifier).to.not.equal(notifier2);
         });
 
         it("can notify a change on an object", () => {