fix(core): improve detection changes of templateOptions props (#2971)

fix #2961

UPGRADE-6.0.md

@@ -4,6 +4,45 @@ UPGRADE FROM 5.0 to 6.0
 
 @ngx-formly/core
 ----------------
+- All Formly components now use `OnPush` change detection, so in order to let Angular detect changes of `templateOptions` properties either ensure a default value is set or use spread object assign instead of regular assign:
+
+  ### Solution 1: set a default value
+
+  ```patch
+  export class CustomFieldType extends FieldType {
+    defaultOptions = {
+      templateOptions: {
+  +     loading: false,
+      },
+    };
+
+    showLoader() {
+      field.templateOptions.loading = true
+    }
+  }
+  ```
+
+  ### Solution 2: use spread object assign
+
+  ```patch
+    showLoader() {
+  -   field.templateOptions.loading = true
+  +   field.templateOptions.loading = {
+  +     ...field.templateOptions,
+  +     loading: true
+  +   };
+    }
+  ```
+
+  - **Note**:
+    The above changes concern only the extra properties defined in your custom type and not the provided ones from Formly such as `disabled`, `label` ...
+
+    ```ts
+    // still working in `V6`
+    field.templateOptions.disabled = true;
+    ```
+
+
 - The defaultValue for fieldGroup and fieldArray has been changed to `undefined` instead of empty object. ([#1901](https://github.com/ngx-formly/ngx-formly/pull/1901), if you want to rely on the old behavior set the `defaultValue`:
 
   **before**:  

src/core/src/lib/components/formly.field.ts

@@ -242,18 +242,8 @@ export class FormlyField implements DoCheck, OnInit, OnChanges, AfterContentInit
     }
 
     const subscribes = [
-      observeDeep({
-        source: field,
-        target: field.templateOptions,
-        paths: ['templateOptions'],
-        setFn: () => field.options.detectChanges(field),
-      }),
-      observeDeep({
-        source: field,
-        target: field.options.formState,
-        paths: ['options', 'formState'],
-        setFn: () => field.options.detectChanges(field),
-      }),
+      observeDeep(field, ['templateOptions'], () => field.options.detectChanges(field)),
+      observeDeep(field, ['options', 'formState'], () => field.options.detectChanges(field)),
     ];
 
     for (const path of [['template'], ['fieldGroupClassName'], ['validation', 'show']]) {

src/core/src/lib/utils.spec.ts

@@ -8,6 +8,7 @@ import {
   observe,
   assignFieldValue,
   defineHiddenProp,
+  observeDeep,
 } from './utils';
 import { FormlyFieldConfig } from './models';
 import { of } from 'rxjs';
@@ -250,6 +251,54 @@ describe('clone', () => {
   });
 });
 
+describe('observeDeep', () => {
+  it('should not emit first change on observe', () => {
+    const spy = jest.fn();
+    observeDeep({ foo: 'test' }, ['foo'], spy);
+    expect(spy).not.toHaveBeenCalled();
+  });
+
+  it('should observe a scalar value', () => {
+    const spy = jest.fn();
+    const o = { foo: 'test' };
+    observeDeep(o, ['foo'], spy);
+    o.foo = 'bar';
+
+    expect(spy).toHaveBeenCalledTimes(1);
+  });
+
+  it('should observe an object', () => {
+    const spy = jest.fn();
+    const o = { address: { city: 'foo' } };
+    observeDeep(o, ['address'], spy);
+    const prevAddress = o.address;
+    o.address = { city: 'foo' };
+    o.address.city = 'bar';
+
+    expect(spy).toHaveBeenCalledTimes(2);
+
+    // check if unsubscribed from the old object
+    spy.mockReset();
+    prevAddress.city = 'bar';
+    expect(spy).toHaveBeenCalledTimes(0);
+  });
+
+  it('should be able to unsubscribe', () => {
+    const spy = jest.fn();
+    const o = { address: { city: 'foo' } };
+    const unsubscribe = observeDeep(o, ['address'], spy);
+    const prevAddress = o.address;
+    o.address.city = 'bar';
+    expect(spy).toHaveBeenCalledTimes(1);
+
+    unsubscribe();
+    spy.mockReset();
+    o.address.city = 'test';
+
+    expect(spy).toHaveBeenCalledTimes(0);
+  });
+});
+
 describe('observe', () => {
   it('should emit first change on observe', () => {
     const spy = jest.fn();

src/core/src/lib/utils.ts

@@ -225,34 +225,27 @@ interface IObserveTarget<T> {
   };
 }
 
-export function observeDeep({ source, paths, target, setFn }) {
-  const observers = [];
-  if (paths.length === 0) {
-    target = source;
-  }
-
-  Object.keys(target).forEach((path) => {
-    let unsubscribe = () => {};
-    const observer = observe(source, [...paths, path], ({ firstChange, currentValue }) => {
-      !firstChange && setFn();
+export function observeDeep(source: any, paths: string[], setFn: () => void): () => void {
+  let observers = [];
 
-      unsubscribe();
-      const i = observers.indexOf(unsubscribe);
-      if (i > -1) {
-        observers.splice(i, 1);
-      }
-
-      if (isObject(currentValue) && currentValue.constructor.name === 'Object') {
-        unsubscribe = observeDeep({ source, setFn, paths: [...paths, path], target: currentValue });
-        observers.push(unsubscribe);
-      }
-    });
+  const unsubscribe = () => {
+    observers.forEach((observer) => observer());
+    observers = [];
+  };
+  const observer = observe(source, paths, ({ firstChange, currentValue }) => {
+    !firstChange && setFn();
 
-    observers.push(() => observer.unsubscribe());
+    unsubscribe();
+    if (isObject(currentValue) && currentValue.constructor.name === 'Object') {
+      Object.keys(currentValue).forEach((prop) => {
+        observers.push(observeDeep(source, [...paths, prop], setFn));
+      });
+    }
   });
 
   return () => {
-    observers.forEach((observer) => observer());
+    observer.unsubscribe();
+    unsubscribe();
   };
 }
 
@@ -279,7 +272,7 @@ export function observe<T = any>(o: IObserveTarget<T>, paths: string[], setFn: I
   if (state.onChange.indexOf(setFn) === -1) {
     state.onChange.push(setFn);
     setFn({ currentValue: state.value, firstChange: true });
-    if (state.onChange.length === 1) {
+    if (state.onChange.length >= 1) {
       const { enumerable } = Object.getOwnPropertyDescriptor(target, key) || { enumerable: true };
       Object.defineProperty(target, key, {
         enumerable,