fix(fast-element): do not notify splices for changes pre-subscription…

… (v2) (#6006)

* fix(fast-element): do not notify splices for changes pre-subscription

* Change files

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

change/@microsoft-fast-element-65662448-c859-4bfd-b06d-a52379348361.json

@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "fix(fast-element): do not notify splices for changes pre-subscription",
+  "packageName": "@microsoft/fast-element",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}

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

@@ -336,6 +336,24 @@ describe("The ArrayObserver", () => {
         expect(changeArgs![0].removed).members([]);
         expect(changeArgs![0].index).equal(0);
     });
+
+    it("should not deliver splices for changes prior to subscription", async () => {
+        ArrayObserver.enable();
+        const array = [1,2,3,4,5];
+        const observer = Observable.getNotifier(array);
+        let wasCalled = false;
+
+        array.push(6);
+        observer.subscribe({
+            handleChange() {
+                wasCalled = true;
+            }
+        });
+
+        await Updates.next();
+
+        expect(wasCalled).to.be.false;
+    })
 });
 
 describe("The array length observer", () => {

packages/web-components/fast-element/src/observation/arrays.ts

@@ -423,6 +423,11 @@ class DefaultArrayObserver extends SubscriberSet implements ArrayObserver {
         setNonEnumerable(subject, "$fastController", this);
     }
 
+    public subscribe(subscriber: Subscriber): void {
+        this.flush();
+        super.subscribe(subscriber);
+    }
+
     public addSplice(splice: Splice) {
         if (this.splices === void 0) {
             this.splices = [splice];

packages/web-components/fast-element/src/observation/notifier.ts

@@ -49,39 +49,6 @@ export interface Notifier {
     unsubscribe(subscriber: Subscriber, propertyToUnwatch?: any): void;
 }
 
-const spilloverMethods = {
-    subscribe(this: SubscriberSet, subscriber: Subscriber): void {
-        const spillover = (this as any).spillover as Subscriber[];
-        const index = spillover.indexOf(subscriber);
-
-        if (index === -1) {
-            spillover.push(subscriber);
-        }
-    },
-
-    unsubscribe(this: SubscriberSet, subscriber: Subscriber): void {
-        const spillover = (this as any).spillover as Subscriber[];
-        const index = spillover.indexOf(subscriber);
-
-        if (index !== -1) {
-            spillover.splice(index, 1);
-        }
-    },
-
-    notify(this: SubscriberSet, args: any): void {
-        const spillover = (this as any).spillover as Subscriber[];
-        const subject = this.subject;
-
-        for (let i = 0, ii = spillover.length; i < ii; ++i) {
-            spillover[i].handleChange(subject, args);
-        }
-    },
-
-    has(this: SubscriberSet, subscriber: Subscriber): boolean {
-        return ((this as any).spillover as Subscriber[]).indexOf(subscriber) !== -1;
-    },
-};
-
 /**
  * An implementation of {@link Notifier} that efficiently keeps track of
  * subscribers interested in a specific change notification on an
@@ -118,43 +85,62 @@ export class SubscriberSet implements Notifier {
      * @param subscriber - The subscriber to test for inclusion in this set.
      */
     public has(subscriber: Subscriber): boolean {
-        return this.sub1 === subscriber || this.sub2 === subscriber;
+        return this.spillover === void 0
+            ? this.sub1 === subscriber || this.sub2 === subscriber
+            : this.spillover.indexOf(subscriber) !== -1;
     }
 
     /**
      * Subscribes to notification of changes in an object's state.
      * @param subscriber - The object that is subscribing for change notification.
      */
     public subscribe(subscriber: Subscriber): void {
-        if (this.has(subscriber)) {
-            return;
-        }
+        const spillover = this.spillover;
 
-        if (this.sub1 === void 0) {
-            this.sub1 = subscriber;
-            return;
-        }
+        if (spillover === void 0) {
+            if (this.has(subscriber)) {
+                return;
+            }
 
-        if (this.sub2 === void 0) {
-            this.sub2 = subscriber;
-            return;
-        }
+            if (this.sub1 === void 0) {
+                this.sub1 = subscriber;
+                return;
+            }
 
-        this.spillover = [this.sub1, this.sub2, subscriber];
-        Object.assign(this, spilloverMethods);
-        this.sub1 = void 0;
-        this.sub2 = void 0;
+            if (this.sub2 === void 0) {
+                this.sub2 = subscriber;
+                return;
+            }
+
+            this.spillover = [this.sub1, this.sub2, subscriber];
+            this.sub1 = void 0;
+            this.sub2 = void 0;
+        } else {
+            const index = spillover.indexOf(subscriber);
+            if (index === -1) {
+                spillover.push(subscriber);
+            }
+        }
     }
 
     /**
      * Unsubscribes from notification of changes in an object's state.
      * @param subscriber - The object that is unsubscribing from change notification.
      */
     public unsubscribe(subscriber: Subscriber): void {
-        if (this.sub1 === subscriber) {
-            this.sub1 = void 0;
-        } else if (this.sub2 === subscriber) {
-            this.sub2 = void 0;
+        const spillover = this.spillover;
+
+        if (spillover === void 0) {
+            if (this.sub1 === subscriber) {
+                this.sub1 = void 0;
+            } else if (this.sub2 === subscriber) {
+                this.sub2 = void 0;
+            }
+        } else {
+            const index = spillover.indexOf(subscriber);
+            if (index !== -1) {
+                spillover.splice(index, 1);
+            }
         }
     }
 
@@ -163,12 +149,24 @@ export class SubscriberSet implements Notifier {
      * @param args - Data passed along to subscribers during notification.
      */
     public notify(args: any): void {
-        if (this.sub1 !== void 0) {
-            this.sub1.handleChange(this.subject, args);
-        }
+        const spillover = this.spillover;
+        const subject = this.subject;
 
-        if (this.sub2 !== void 0) {
-            this.sub2.handleChange(this.subject, args);
+        if (spillover === void 0) {
+            const sub1 = this.sub1;
+            const sub2 = this.sub2;
+
+            if (sub1 !== void 0) {
+                sub1.handleChange(subject, args);
+            }
+
+            if (sub2 !== void 0) {
+                sub2.handleChange(subject, args);
+            }
+        } else {
+            for (let i = 0, ii = spillover.length; i < ii; ++i) {
+                spillover[i].handleChange(subject, args);
+            }
         }
     }
 }