fix(engine): issue #858 to make setters reactive cleanup

packages/@lwc/engine/src/framework/component.ts

@@ -88,13 +88,12 @@ function getObservingTemplateRecord(vm: VM): ObservingRecord {
             if (process.env.NODE_ENV !== 'production') {
                 assert.invariant(!isRendering, `Mutating property is not allowed during the rendering life-cycle of ${vmBeingRendered}.`);
             }
-            const { vm } = this;
             if (!vm.isDirty) {
                 markComponentAsDirty(vm);
                 scheduleRehydration(vm);
             }
         },
-    }
+    };
 }
 
 export function renderComponent(vm: VM): VNodes {

packages/@lwc/engine/src/framework/decorators/api.ts

@@ -104,30 +104,30 @@ function createPublicPropertyDescriptor(proto: ComponentConstructor, key: Proper
 }
 
 interface ObservingAccessorRecord extends ObservingRecord {
-    value?: any,
-    debounced: boolean,
+    value?: any;
+    debounced: boolean;
 }
 
 function getObservingAccessorRecord(vm: VM, set: (v: any) => void): ObservingAccessorRecord {
-    let record = vm.oar;
-    if (!isNull(record)) {
-        return record as ObservingAccessorRecord;
-    }
-    return (vm.oar = {
+    return {
         vm,
         listeners: [],
         notify(this: ObservingAccessorRecord) {
             if (isFalse(this.debounced)) {
+                this.debounced = true;
                 addCallbackToNextTick(() => {
                     if (!isNull(this.debounced)) {
+                        set.call(vm.component, this.value);
+                        // debouncing after the call to the original setter to prevent
+                        // infinite cycles if the setter itself is mutating things that
+                        // were accessed during the previous invocation.
                         this.debounced = false;
-                        set.call(this.vm.component, this.value);
                     }
                 });
             }
         },
         debounced: false,
-    } as ObservingAccessorRecord);
+    } as ObservingAccessorRecord;
 }
 
 function createPublicAccessorDescriptor(Ctor: ComponentConstructor, key: PropertyKey, descriptor: PropertyDescriptor): PropertyDescriptor {
@@ -146,7 +146,7 @@ function createPublicAccessorDescriptor(Ctor: ComponentConstructor, key: Propert
             }
             return get.call(this);
         },
-        set: function setter(this: ComponentInterface, newValue: any) {
+        set(this: ComponentInterface, newValue: any) {
             const vm = getComponentVM(this);
             if (process.env.NODE_ENV !== 'production') {
                 assert.isTrue(vm && "cmpRoot" in vm, `${vm} is not a vm.`);
@@ -176,18 +176,19 @@ function createPublicAccessorDescriptor(Ctor: ComponentConstructor, key: Propert
                 let error;
                 const v = reactiveMembrane.getReadOnlyProxy(newValue);
                 const currentObservingRecordInception = getCurrentObservingRecord();
-                if (!isNull(vm.oar)) {
-                    resetObservingRecord(vm.oar);
+                let oRecord = vm.oar[key as any];
+                if (!isUndefined(oRecord)) {
+                    resetObservingRecord(oRecord);
                 } else {
-                    vm.oar = getObservingAccessorRecord(vm, set) as ObservingAccessorRecord;
+                    oRecord = vm.oar[key as any] = getObservingAccessorRecord(vm, set) as ObservingAccessorRecord;
                 }
-                setCurrentObservingRecord(vm.oar);
+                setCurrentObservingRecord(oRecord);
                 // every time we invoke this setter from outside (thru this wrapper setter)
                 // we should reset the value and the debounced just in case there is a pending
                 // invocation the next tick that is not longer relevant since the value is changing
                 // from outside.
-                (vm.oar as ObservingAccessorRecord).value = v;
-                (vm.oar as ObservingAccessorRecord).debounced = false;
+                (oRecord as ObservingAccessorRecord).value = v;
+                (oRecord as ObservingAccessorRecord).debounced = false;
                 try {
                     set.call(this, v);
                 } catch (e) {

packages/@lwc/engine/src/framework/vm.ts

@@ -56,9 +56,10 @@ export interface VM {
     fallback: boolean;
     mode: string;
     component?: ComponentInterface;
-    /** observed mutation entry */
+    /** Observing Record for values used by the selected template */
     otr: ObservingRecord | null;
-    oar: ObservingRecord | null;
+    /** Observing Records for each of the public @api accessors */
+    oar: Record<PropertyKey, ObservingRecord>;
     toString(): string;
 }
 
@@ -157,11 +158,16 @@ export function removeVM(vm: VM) {
         return; // already removed
     }
     removeInsertionIndex(vm);
+    const { oar, otr } = vm;
     // Just in case it comes back, with this we guarantee re-rendering it
     vm.isDirty = true;
     // Making sure that any observing record will not trigger the rehydrated on this vm
-    if (!isNull(vm.otr)) {
-        resetObservingRecord(vm.otr as ObservingRecord);
+    if (!isNull(otr)) {
+        resetObservingRecord(otr as ObservingRecord);
+    }
+    // Making sure that any observing accessor record will not trigger the setter to be reinvoked
+    for (const key in oar) {
+        resetObservingRecord(oar[key]);
     }
 
     // At this point we need to force the removal of all children because
@@ -214,7 +220,7 @@ export function createVM(tagName: string, elm: HTMLElement, Ctor: ComponentConst
         component: undefined,
         children: EmptyArray,
         otr: null,
-        oar: null,
+        oar: create(null),
     };
 
     if (process.env.NODE_ENV !== 'production') {
@@ -401,7 +407,7 @@ function destroyChildren(children: VNodes) {
         } catch (e) {
             if (process.env.NODE_ENV !== 'production') {
                 const vm = getCustomElementVM(elm as HTMLElement);
-                assert.logError(`Internal Error: Failed to disconnect component ${vm}. ${e} ${e.stack}`, elm as Element);
+                assert.logError(`Internal Error: Failed to disconnect component ${vm}. ${e}`, elm as Element);
             }
         }
     }

packages/@lwc/engine/src/framework/watcher.ts

@@ -15,10 +15,10 @@ const TargetToReactiveRecordMap: WeakMap<object, ReactiveRecord> = new WeakMap()
  * - Accessor Invocation Phase
  */
 export interface ObservingRecord {
-    readonly vm: VM,
-    readonly listeners: ObservedMemberPropertyRecords[],
-    notify(): void,
-};
+    readonly vm: VM;
+    readonly listeners: ObservedMemberPropertyRecords[];
+    notify(): void;
+}
 
 /**
  * An Observed Membre Property Record represents the list of all Observing Records,
@@ -72,7 +72,7 @@ export function observeMutation(target: object, key: PropertyKey) {
         return;
     }
     const oRecord = currentObservingRecord;
-    let reactiveRecord = getReactiveRecord(target);
+    const reactiveRecord = getReactiveRecord(target);
     let observingRecords = reactiveRecord[key as any];
     if (isUndefined(observingRecords)) {
         observingRecords = [];