fix(engine): reactive setters behind a flag (#1444)

* fix(engine): reactive setters behind a flag

* test: enable reactive setter flag

packages/@lwc/engine/src/framework/base-lightning-element.ts

@@ -267,6 +267,7 @@ function BaseLightningElementConstructor(this: LightningElement) {
     const component = this;
     vm.component = component;
     vm.tro = getTemplateReactiveObserver(vm as VM);
+    vm.oar = create(null);
     // interaction hooks
     // We are intentionally hiding this argument from the formal API of LWCElement because
     // we don't want folks to know about it just yet.

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

@@ -4,13 +4,15 @@
  * SPDX-License-Identifier: MIT
  * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT
  */
-import { assert, isFalse, isFunction, isObject, isUndefined, toString } from '@lwc/shared';
+import { ENABLE_REACTIVE_SETTER } from '@lwc/features';
+import { assert, isFalse, isFunction, isObject, isTrue, isUndefined, toString } from '@lwc/shared';
 import { logError } from '../../shared/assert';
 import { isRendering, vmBeingRendered, isBeingConstructed } from '../invoker';
-import { valueObserved, valueMutated } from '../../libs/mutation-tracker';
+import { valueObserved, valueMutated, ReactiveObserver } from '../../libs/mutation-tracker';
 import { ComponentInterface, ComponentConstructor } from '../component';
-import { getComponentVM } from '../vm';
+import { getComponentVM, rerenderVM } from '../vm';
 import { getDecoratorsRegisteredMeta } from './register';
+import { addCallbackToNextTick } from '../utils';
 
 /**
  * @api decorator to mark public fields and public methods in
@@ -106,6 +108,42 @@ function createPublicPropertyDescriptor(
     };
 }
 
+class AccessorReactiveObserver extends ReactiveObserver {
+    private value: any;
+    private debouncing: boolean = false;
+    constructor(vm, set) {
+        super(() => {
+            if (isFalse(this.debouncing)) {
+                this.debouncing = true;
+                addCallbackToNextTick(() => {
+                    if (isTrue(this.debouncing)) {
+                        const { value } = this;
+                        const { isDirty: dirtyStateBeforeSetterCall, component, idx } = vm;
+                        set.call(component, value);
+                        // de-bouncing after the call to the original setter to prevent
+                        // infinity loop if the setter itself is mutating things that
+                        // were accessed during the previous invocation.
+                        this.debouncing = false;
+                        if (isTrue(vm.isDirty) && isFalse(dirtyStateBeforeSetterCall) && idx > 0) {
+                            // immediate rehydration due to a setter driven mutation, otherwise
+                            // the component will get rendered on the second tick, which it is not
+                            // desirable.
+                            rerenderVM(vm);
+                        }
+                    }
+                });
+            }
+        });
+    }
+    reset(value?: any) {
+        super.reset();
+        this.debouncing = false;
+        if (arguments.length > 0) {
+            this.value = value;
+        }
+    }
+}
+
 function createPublicAccessorDescriptor(
     Ctor: ComponentConstructor,
     key: PropertyKey,
@@ -144,7 +182,22 @@ function createPublicAccessorDescriptor(
                 );
             }
             if (set) {
-                set.call(this, newValue);
+                if (ENABLE_REACTIVE_SETTER) {
+                    let ro = vm.oar[key as any] as AccessorReactiveObserver;
+                    if (isUndefined(ro)) {
+                        ro = vm.oar[key as any] = new AccessorReactiveObserver(vm, set);
+                    }
+                    // every time we invoke this setter from outside (through this wrapper setter)
+                    // we should reset the value and then debounce just in case there is a pending
+                    // invocation the next tick that is not longer relevant since the value is changing
+                    // from outside.
+                    ro.reset(newValue);
+                    ro.observe(() => {
+                        set.call(this, newValue);
+                    });
+                } else {
+                    set.call(this, newValue);
+                }
             } else if (process.env.NODE_ENV !== 'production') {
                 assert.fail(
                     `Invalid attempt to set a new value for property ${toString(

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

@@ -107,6 +107,7 @@ export interface UninitializedVM {
     component?: ComponentInterface;
     cmpRoot?: ShadowRoot;
     tro?: ReactiveObserver;
+    oar?: Record<PropertyKey, ReactiveObserver>;
 }
 
 export interface VM extends UninitializedVM {
@@ -115,6 +116,8 @@ export interface VM extends UninitializedVM {
     cmpRoot: ShadowRoot;
     /** Template Reactive Observer to observe values used by the selected template */
     tro: ReactiveObserver;
+    /** Reactive Observers for each of the public @api accessors */
+    oar: Record<PropertyKey, ReactiveObserver>;
 }
 
 let idx: number = 0;
@@ -167,9 +170,13 @@ function resetComponentStateWhenRemoved(vm: VM) {
     }
     const { state } = vm;
     if (state !== VMState.disconnected) {
-        const { tro } = vm;
+        const { oar, tro } = vm;
         // Making sure that any observing record will not trigger the rehydrated on this vm
         tro.reset();
+        // Making sure that any observing accessor record will not trigger the setter to be reinvoked
+        for (const key in oar) {
+            oar[key].reset();
+        }
         runDisconnectedCallback(vm);
         // Spec: https://dom.spec.whatwg.org/#concept-node-remove (step 14-15)
         runShadowChildNodesDisconnectedCallback(vm);
@@ -243,6 +250,7 @@ export function createVM(elm: HTMLElement, Ctor: ComponentConstructor, options:
         component: undefined,
         cmpRoot: undefined,
         tro: undefined,
+        oar: undefined,
     };
 
     if (process.env.NODE_ENV !== 'production') {

packages/@lwc/features/src/flags.ts

@@ -9,5 +9,6 @@ export type FeatureFlag = boolean | null;
 export const ENABLE_FOO: FeatureFlag = true;
 export const ENABLE_BAR: FeatureFlag = false;
 export const ENABLE_BAZ: FeatureFlag = null;
+export const ENABLE_REACTIVE_SETTER: FeatureFlag = null;
 
 export { runtimeFlags, setFeatureFlag, setFeatureFlagForTest } from './runtime';

packages/integration-karma/test/reactivity/setters/index.spec.js

@@ -1,9 +1,10 @@
-import { createElement } from 'test-utils';
+import { createElement, setFeatureFlagForTest } from 'lwc';
 
 import Parent from 'x/parent';
 
-// TODO: issue #858 to enable reactive setters
-xdescribe('Reactivity for setters', () => {
+describe('Reactivity for setters', () => {
+    setFeatureFlagForTest('ENABLE_REACTIVE_SETTER', true);
+
     it('should not alter rendering if they are not invoked', () => {
         const elm = createElement('x-test', { is: Parent });
         document.body.appendChild(elm);