Merge pull request #38 from cloudnc/fix/emit-form-changes

Fix/emit form changes

README.md

@@ -63,7 +63,7 @@ interface OneListingForm {
   title: string;
   price: number;
 
-  // polymorphic form where product can either be a vehicule or a droid
+  // polymorphic form where product can either be a vehicle or a droid
   listingType: ListingType | null;
   vehicleProduct: OneVehicle | null;
   droidProduct: OneDroid | null;
@@ -294,7 +294,7 @@ export class VehicleProductComponent extends NgxSubFormRemapComponent<OneVehicle
 }
 ```
 
-Our "incoming" object is of type `OneVehicle` but into that component we treat it as a `OneVehicleForm` to split the vehicule (either a `speeder` or `spaceship`) in 2 **separated** properties.
+Our "incoming" object is of type `OneVehicle` but into that component we treat it as a `OneVehicleForm` to split the vehicle (either a `speeder` or `spaceship`) in 2 **separated** properties.
 
 ### Helpers
 

cypress/helpers/data.helper.ts

@@ -11,8 +11,8 @@ export interface ListElement {
   readonly details: string;
 }
 
-export type VehiculeFormElement = {
-  readonly vehiculeType: string;
+export type VehicleFormElement = {
+  readonly vehicleType: string;
 } & (
   | {
       readonly spaceshipForm: {
@@ -50,7 +50,7 @@ export interface FormElement {
     readonly imageUrl: string;
     readonly price: string;
     readonly listingType: string;
-  } & ({ readonly vehiculeForm: VehiculeFormElement });
+  } & ({ readonly vehicleForm: VehicleFormElement });
 }
 
 export const hardcodedElementToTestElement = (item: OneListing): ListElement => {

cypress/helpers/dom.helper.ts

@@ -89,7 +89,7 @@ export const DOM = {
         return cy.get(`*[data-no-error]`);
       },
       get obj(): Cypress.Chainable<FormElement> {
-        const getVehiculeObj = (element: HTMLElement, type: VehicleType) =>
+        const getVehicleObj = (element: HTMLElement, type: VehicleType) =>
           ({
             Spaceship: {
               spaceshipForm: {
@@ -120,12 +120,9 @@ export const DOM = {
                 imageUrl: getTextFromInput(element, 'input-image-url'),
                 price: getTextFromInput(element, 'input-price'),
                 listingType: getSelectedOptionFromSelect(element, 'select-listing-type'),
-                vehiculeForm: {
-                  vehiculeType: getSelectedOptionFromSelect(element, 'select-vehicule-type'),
-                  ...getVehiculeObj(element, getSelectedOptionFromSelect(
-                    element,
-                    'select-vehicule-type',
-                  ) as VehicleType),
+                vehicleForm: {
+                  vehicleType: getSelectedOptionFromSelect(element, 'select-vehicle-type'),
+                  ...getVehicleObj(element, getSelectedOptionFromSelect(element, 'select-vehicle-type') as VehicleType),
                 },
               },
             }))

package.json

@@ -25,7 +25,8 @@
     "lib:test:ci": "yarn run ng test --project ngx-sub-form --watch false",
     "------------------ Quick Commands ------------------": "",
     "lint:fix": "yarn demo:lint:fix && yarn prettier:write",
-    "semantic-release": "semantic-release"
+    "semantic-release": "semantic-release",
+    "test": "yarn lib:test:watch"
   },
   "private": true,
   "resolutions": {

projects/ngx-sub-form/src/lib/ngx-sub-form.component.spec.ts

@@ -306,38 +306,59 @@ describe(`NgxSubFormComponent`, () => {
         done();
       }, 0);
     });
+
+    it(`should correctly emit the onChange value only once when form is patched locally`, (done: () => void) => {
+      const spyOnFormUpdate = jasmine.createSpy();
+      const spyOnChange = jasmine.createSpy();
+      subComponent.onFormUpdate = spyOnFormUpdate;
+      subComponent.registerOnChange(spyOnChange);
+      (subComponent as any).emitInitialValueOnInit = false;
+
+      subComponent.formGroup.patchValue({ color: 'red', canFire: false });
+
+      setTimeout(() => {
+        expect(spyOnFormUpdate).toHaveBeenCalledWith({
+          canFire: true,
+        });
+
+        expect(spyOnChange).toHaveBeenCalledTimes(1);
+        expect(spyOnChange).toHaveBeenCalledWith({ color: 'red', canFire: false, numberOfPeopleOnBoard: 10 });
+
+        done();
+      }, 0);
+    });
   });
 });
 
-interface VehiculeForm {
-  vehiculeColor: Vehicle['color'] | null;
-  vehiculeCanFire: Vehicle['canFire'] | null;
-  vehiculeNumberOfPeopleOnBoard: Vehicle['numberOfPeopleOnBoard'] | null;
+interface VehicleForm {
+  vehicleColor: Vehicle['color'] | null;
+  vehicleCanFire: Vehicle['canFire'] | null;
+  vehicleNumberOfPeopleOnBoard: Vehicle['numberOfPeopleOnBoard'] | null;
 }
 
-class SubRemapComponent extends NgxSubFormRemapComponent<Vehicle, VehiculeForm> {
+class SubRemapComponent extends NgxSubFormRemapComponent<Vehicle, VehicleForm> {
   getFormControls() {
-    // even though optional, if we comment out vehiculeColor there should be a TS error
+    // even though optional, if we comment out vehicleColor there should be a TS error
     return {
-      vehiculeColor: new FormControl(getDefaultValues().color),
-      vehiculeCanFire: new FormControl(getDefaultValues().canFire),
-      vehiculeNumberOfPeopleOnBoard: new FormControl(getDefaultValues().numberOfPeopleOnBoard),
+      vehicleColor: new FormControl(getDefaultValues().color),
+      vehicleCanFire: new FormControl(getDefaultValues().canFire),
+      vehicleNumberOfPeopleOnBoard: new FormControl(getDefaultValues().numberOfPeopleOnBoard),
     };
   }
 
-  protected transformToFormGroup(obj: Vehicle | null): VehiculeForm {
+  protected transformToFormGroup(obj: Vehicle | null): VehicleForm {
     return {
-      vehiculeColor: obj ? obj.color : null,
-      vehiculeCanFire: obj ? obj.canFire : null,
-      vehiculeNumberOfPeopleOnBoard: obj ? obj.numberOfPeopleOnBoard : null,
+      vehicleColor: obj ? obj.color : null,
+      vehicleCanFire: obj ? obj.canFire : null,
+      vehicleNumberOfPeopleOnBoard: obj ? obj.numberOfPeopleOnBoard : null,
     };
   }
 
-  protected transformFromFormGroup(formValue: VehiculeForm): Vehicle | null {
+  protected transformFromFormGroup(formValue: VehicleForm): Vehicle | null {
     return {
-      color: formValue.vehiculeColor,
-      canFire: formValue.vehiculeCanFire,
-      numberOfPeopleOnBoard: formValue.vehiculeNumberOfPeopleOnBoard,
+      color: formValue.vehicleColor,
+      canFire: formValue.vehicleCanFire,
+      numberOfPeopleOnBoard: formValue.vehicleNumberOfPeopleOnBoard,
     };
   }
 }
@@ -363,9 +384,9 @@ describe(`NgxSubFormRemapComponent`, () => {
 
         expect(subRemapComponent.formGroup.setValue).toHaveBeenCalledWith(
           {
-            vehiculeColor: getDefaultValues().color,
-            vehiculeCanFire: getDefaultValues().canFire,
-            vehiculeNumberOfPeopleOnBoard: getDefaultValues().numberOfPeopleOnBoard,
+            vehicleColor: getDefaultValues().color,
+            vehicleCanFire: getDefaultValues().canFire,
+            vehicleNumberOfPeopleOnBoard: getDefaultValues().numberOfPeopleOnBoard,
           },
           {
             emitEvent: false,
@@ -395,14 +416,20 @@ describe(`NgxSubFormRemapComponent`, () => {
 
         onChangeSpy.calls.reset();
 
+        const newExpectation = {
+          color: 'green',
+          canFire: false,
+          numberOfPeopleOnBoard: 12,
+        };
+
         subRemapComponent.formGroup.setValue({
-          vehiculeColor: getDefaultValues().color,
-          vehiculeCanFire: getDefaultValues().canFire,
-          vehiculeNumberOfPeopleOnBoard: getDefaultValues().numberOfPeopleOnBoard,
+          vehicleColor: newExpectation.color,
+          vehicleCanFire: newExpectation.canFire,
+          vehicleNumberOfPeopleOnBoard: newExpectation.numberOfPeopleOnBoard,
         });
 
         setTimeout(() => {
-          expect(onChangeSpy).toHaveBeenCalledWith(expectedValue);
+          expect(onChangeSpy).toHaveBeenCalledWith(newExpectation);
 
           done();
         }, 0);

projects/ngx-sub-form/src/lib/ngx-sub-form.component.ts

@@ -1,9 +1,8 @@
 import { OnDestroy } from '@angular/core';
-import { ControlValueAccessor, FormGroup, ValidationErrors, Validator, AbstractControl } from '@angular/forms';
-import { Subscription, combineLatest, merge, Observable } from 'rxjs';
-import { delay, filter, startWith, map, withLatestFrom } from 'rxjs/operators';
+import { AbstractControl, ControlValueAccessor, FormGroup, ValidationErrors, Validator } from '@angular/forms';
+import { merge, Observable, Subscription } from 'rxjs';
+import { delay, filter, map, startWith, withLatestFrom } from 'rxjs/operators';
 import { ControlMap, Controls, ControlsNames, FormUpdate } from './ngx-sub-form-utils';
-import { keyValuePairToObj } from '../helpers/utils';
 
 interface OnFormUpdate<FormInterface> {
   onFormUpdate?: (formUpdate: FormUpdate<FormInterface>) => void;
@@ -180,18 +179,18 @@ export abstract class NgxSubFormComponent<ControlInterface, FormInterface = Cont
       ),
     );
 
-    const keyLastEmit$: Observable<keyof FormInterface> = merge(...formValues.map(obs => obs.pipe(map(x => x.key))));
+    const lastKeyEmitted$: Observable<keyof FormInterface> = merge(...formValues.map(obs => obs.pipe(map(x => x.key))));
 
-    this.subscription = combineLatest<KeyValueForm[]>(...formValues)
+    this.subscription = this.formGroup.valueChanges
       .pipe(
+        startWith(this.formGroup.value),
         filter(() => !!this.formGroup),
         // this is required otherwise an `ExpressionChangedAfterItHasBeenCheckedError` will happen
         // this is due to the fact that parent component will define a given state for the form that might
         // be changed once the children are being initialized
         delay(0),
-        map(x => keyValuePairToObj<FormInterface>(x)),
         // detect which stream emitted last
-        withLatestFrom(keyLastEmit$),
+        withLatestFrom(lastKeyEmitted$),
         map(([changes, keyLastEmit], index) => {
           if (index > 0 && this.onTouched) {
             this.onTouched();

src/app/app.spec.e2e.ts

@@ -31,8 +31,8 @@ context(`EJawa demo`, () => {
         imageUrl: x.imageUrl,
         price: x.price + '',
         listingType: x.listingType,
-        vehiculeForm: {
-          vehiculeType: x.product.vehicleType,
+        vehicleForm: {
+          vehicleType: x.product.vehicleType,
           spaceshipForm: {
             color: v.color,
             canFire: v.canFire,
@@ -62,8 +62,8 @@ context(`EJawa demo`, () => {
         imageUrl: x.imageUrl,
         price: x.price + '',
         listingType: x.listingType,
-        vehiculeForm: {
-          vehiculeType: x.product.vehicleType,
+        vehicleForm: {
+          vehicleType: x.product.vehicleType,
           speederForm: {
             color: v.color,
             canFire: v.canFire,

src/app/main/listing/vehicle-listing/vehicle-product.component.html

@@ -3,7 +3,7 @@
 
   <mat-form-field>
     <mat-select
-      data-select-vehicule-type
+      data-select-vehicle-type
       placeholder="Select vehicle type"
       [formControlName]="formControlNames.vehicleType"
     >