From d9313226221f40c351c5a3265d4d2ac17c202214 Mon Sep 17 00:00:00 2001 From: Maxime Robert Date: Mon, 10 Feb 2020 17:16:34 +0000 Subject: [PATCH] feat(lib): add `NgxRootFormRemapComponent` and `NgxAutomaticRootFormRemapComponent` BREAKING CHANGES: You'll need to be more precise when using `NgxRootFormComponent` and `NgxAutomaticRootFormComponent`. They now have both a `remap` version. It was very easy to forget about the remap methods previously and some bugs could quickly sneak in. This closes cloudnc/ngx-sub-form#133 --- README.md | 71 ++++++++++++++++--- .../lib/ngx-automatic-root-form.component.ts | 21 +++++- .../src/lib/ngx-root-form.component.spec.ts | 6 +- .../src/lib/ngx-root-form.component.ts | 26 ++++--- .../src/lib/ngx-sub-form.decorators.ts | 17 +++-- .../listing-form/listing-form.component.ts | 11 ++- src/readme/listing.component.ts | 54 ++++++++++++-- 7 files changed, 166 insertions(+), 40 deletions(-) diff --git a/README.md b/README.md index ca2d8ebc..26dada62 100644 --- a/README.md +++ b/README.md @@ -42,8 +42,14 @@ If you want to see the demo in action, please visit [https://cloudnc.github.io/n `ngx-sub-form` provides -- 2 classes for top level form components: `NgxRootFormComponent`, `NgxAutomaticRootFormComponent` -- 2 classes for sub level form components: `NgxSubFormComponent`, `NgxSubFormRemapComponent` +- 4 classes for top level form components: + - `NgxRootFormComponent` + - `NgxRootFormRemapComponent` + - `NgxAutomaticRootFormComponent` + - `NgxAutomaticRootFormRemapComponent` +- 2 classes for sub level form components: + - `NgxSubFormComponent` + - `NgxSubFormRemapComponent` - 3 interfaces: `Controls`, `ControlsNames`, `FormGroupOptions` - 1 function: `subformComponentProviders` @@ -64,10 +70,12 @@ So there's actually nothing to setup (like a module), you can just use them dire ### Type safety you said? -When extending one of the 4 core classes: +When extending one of the 6 core classes: - `NgxRootFormComponent` +- `NgxRootFormRemapComponent` - `NgxAutomaticRootFormComponent` +- `NgxAutomaticRootFormRemapComponent` - `NgxSubFormComponent` - `NgxSubFormRemapComponent` @@ -84,19 +92,19 @@ When refactoring your interfaces/classes, your form will error at build time if ### Angular hooks -ngx-sub-form uses `ngOnInit` and `ngOnDestroy` internally. +`ngx-sub-form` uses `ngOnInit` and `ngOnDestroy` internally. If you need to use them too, do not forget to call `super.ngOnInit()` and `super.ngOnDestroy()` otherwise you might end with with the form not working correctly or a memory leak. Unfortunately, there's currently no way of making sure that inheriting classes call these methods, so keep that in mind. ### First component level -Within the component where the (top) form will be handled, you have to define the top level structure. You can do it manually as you'd usually do (by defining your own `FormGroup`), but it's better to extend from either `NgxRootFormComponent` or `NgxAutomaticRootFormComponent` as you'll get some type safety and other useful helpers. If dealing with polymorphic data, **each type must have it's own form control**: +Within the component where the (top) form will be handled, you have to define the top level structure. You can do it manually as you'd usually do (by defining your own `FormGroup`), but it's better to extend from either `NgxRootFormComponent` or `NgxAutomaticRootFormComponent` (or their `remap` versions) as you'll get some type safety and other useful helpers. If dealing with polymorphic data, **each type must have it's own form control**: (_even if it doesn't match your model, we'll talk about that later_) Before explaining the difference between `NgxRootFormComponent` or `NgxAutomaticRootFormComponent`, let's look at an example with a polymorphic type: ```ts -// src/readme/listing.component.ts#L8-L58 +// src/readme/listing.component.ts#L9-L104 enum ListingType { VEHICLE = 'Vehicle', @@ -120,9 +128,9 @@ export interface OneListingForm { templateUrl: './listing.component.html', styleUrls: ['./listing.component.scss'], }) -export class ListingComponent extends NgxAutomaticRootFormComponent { +export class ListingComponent extends NgxAutomaticRootFormRemapComponent { // as we're renaming the input, it'd be impossible for ngx-sub-form to guess - // the name of your input to then check within the `ngOnChanges` hook wheter + // the name of your input to then check within the `ngOnChanges` hook whether // it has been updated or not // another solution would be to ask you to use a setter and call a hook but // this is too verbose, that's why we created a decorator `@DataInput` @@ -148,6 +156,51 @@ export class ListingComponent extends NgxAutomaticRootFormComponent | null, + ): OneListingForm | null { + if (!obj) { + return null; + } + + return { + id: obj.id, + title: obj.title, + price: obj.price, + imageUrl: obj.imageUrl, + + listingType: obj.listingType, + vehicleProduct: obj.listingType === ListingType.VEHICLE ? obj.product : null, + droidProduct: obj.listingType === ListingType.DROID ? obj.product : null, + }; + } + + protected transformFromFormGroup(formValue: OneListingForm): VehicleListing | DroidListing | null { + const { id, title, price, imageUrl, listingType } = formValue; + const base = { id, title, price, imageUrl, listingType }; + + switch (formValue.listingType) { + case null: { + throw new Error(`listingType is set but the corresponding value is null`); + } + case ListingType.DROID: { + if (!formValue.droidProduct) { + throw new Error(`listingType is of type DROID but droidProduct is not defined`); + } + return { ...base, listingType: ListingType.DROID, product: formValue.droidProduct }; + } + case ListingType.VEHICLE: { + if (!formValue.droidProduct) { + throw new Error(`listingType is of type VEHICLE but droidProduct is not defined`); + } + return { ...base, listingType: ListingType.DROID, product: formValue.droidProduct }; + } + default: + throw new UnreachableCase(formValue.listingType); + } + } } ``` @@ -267,7 +320,7 @@ which will require you to define two interfaces: Example, take a look at [`VehicleProductComponent`](https://github.com/cloudnc/ngx-sub-form/blob/master/src/app/main/listing/listing-form/vehicle-listing/vehicle-product.component.ts): ```ts -// src/readme/vehicle-product.component.simplified.ts#L7-L74 +// src/readme/vehicle-product.component.simplified.ts#L7-L73 // merged few files together to make it easier to follow export interface BaseVehicle { diff --git a/projects/ngx-sub-form/src/lib/ngx-automatic-root-form.component.ts b/projects/ngx-sub-form/src/lib/ngx-automatic-root-form.component.ts index 5825db9f..a27d651c 100644 --- a/projects/ngx-sub-form/src/lib/ngx-automatic-root-form.component.ts +++ b/projects/ngx-sub-form/src/lib/ngx-automatic-root-form.component.ts @@ -1,8 +1,8 @@ import { OnInit } from '@angular/core'; -import { NgxRootFormComponent } from './ngx-root-form.component'; +import { NgxRootFormRemapComponent } from './ngx-root-form.component'; -export abstract class NgxAutomaticRootFormComponent - extends NgxRootFormComponent +export abstract class NgxAutomaticRootFormRemapComponent + extends NgxRootFormRemapComponent implements OnInit { /** @internal */ protected onRegisterOnChangeHook(data: ControlInterface | null) { @@ -21,3 +21,18 @@ export abstract class NgxAutomaticRootFormComponent + extends NgxAutomaticRootFormRemapComponent + implements OnInit { + protected transformToFormGroup( + obj: ControlInterface | null, + defaultValues: Partial | null, + ): ControlInterface | null { + return (obj as unknown) as ControlInterface; + } + + protected transformFromFormGroup(formValue: ControlInterface): ControlInterface | null { + return (formValue as unknown) as ControlInterface; + } +} diff --git a/projects/ngx-sub-form/src/lib/ngx-root-form.component.spec.ts b/projects/ngx-sub-form/src/lib/ngx-root-form.component.spec.ts index fc862779..55e97cbf 100644 --- a/projects/ngx-sub-form/src/lib/ngx-root-form.component.spec.ts +++ b/projects/ngx-sub-form/src/lib/ngx-root-form.component.spec.ts @@ -1,9 +1,9 @@ -import { NgxRootFormComponent } from './ngx-root-form.component'; +import { NgxRootFormComponent, NgxRootFormRemapComponent } from './ngx-root-form.component'; import { EventEmitter, Input, Component, Output, DebugElement } from '@angular/core'; import { Controls, ArrayPropertyKey, ArrayPropertyValue } from './ngx-sub-form-utils'; import { FormControl, Validators, ReactiveFormsModule, FormArray } from '@angular/forms'; import { BehaviorSubject } from 'rxjs'; -import { TestBed, async, ComponentFixture, fakeAsync, tick } from '@angular/core/testing'; +import { TestBed, async, ComponentFixture } from '@angular/core/testing'; import { By } from '@angular/platform-browser'; import { DataInput } from './ngx-sub-form.decorators'; import { NgxFormWithArrayControls } from './ngx-sub-form.types'; @@ -186,7 +186,7 @@ interface VehiclesArrayForm { selector: `app-root-form`, template: ``, }) -class RootFormArrayComponent extends NgxRootFormComponent +class RootFormArrayComponent extends NgxRootFormRemapComponent implements NgxFormWithArrayControls { @DataInput() // tslint:disable-next-line:no-input-rename diff --git a/projects/ngx-sub-form/src/lib/ngx-root-form.component.ts b/projects/ngx-sub-form/src/lib/ngx-root-form.component.ts index 9fd7a70d..42d30072 100644 --- a/projects/ngx-sub-form/src/lib/ngx-root-form.component.ts +++ b/projects/ngx-sub-form/src/lib/ngx-root-form.component.ts @@ -5,7 +5,7 @@ import { filter, tap } from 'rxjs/operators'; import { NgxSubFormRemapComponent } from './ngx-sub-form.component'; import { takeUntilDestroyed, isNullOrUndefined } from './ngx-sub-form-utils'; -export abstract class NgxRootFormComponent +export abstract class NgxRootFormRemapComponent extends NgxSubFormRemapComponent implements OnInit { public abstract dataInput: Required | null | undefined; @@ -81,20 +81,24 @@ export abstract class NgxRootFormComponent + extends NgxRootFormRemapComponent + implements OnInit { protected transformToFormGroup( obj: ControlInterface | null, - defaultValues: Partial | null, - ): FormInterface | null { - return (obj as unknown) as FormInterface; + defaultValues: Partial | null, + ): ControlInterface | null { + return (obj as unknown) as ControlInterface; } - protected transformFromFormGroup(formValue: FormInterface): ControlInterface | null { + protected transformFromFormGroup(formValue: ControlInterface): ControlInterface | null { return (formValue as unknown) as ControlInterface; } - - public manualSave(): void { - if (!isNullOrUndefined(this.dataValue) && this.formGroup.valid) { - this._dataOutput$.next(this.dataValue); - } - } } diff --git a/projects/ngx-sub-form/src/lib/ngx-sub-form.decorators.ts b/projects/ngx-sub-form/src/lib/ngx-sub-form.decorators.ts index b06df88d..77fcc5e8 100644 --- a/projects/ngx-sub-form/src/lib/ngx-sub-form.decorators.ts +++ b/projects/ngx-sub-form/src/lib/ngx-sub-form.decorators.ts @@ -1,4 +1,4 @@ -import { NgxRootFormComponent } from './ngx-root-form.component'; +import { NgxRootFormComponent, NgxRootFormRemapComponent } from './ngx-root-form.component'; export class DataInputUsedOnWrongPropertyError extends Error { constructor(calledOnPropertyKey: string) { @@ -8,9 +8,16 @@ export class DataInputUsedOnWrongPropertyError extends Error { } } -export function DataInput() { +export function DataInput(): ( + target: NgxRootFormRemapComponent, + propertyKey: string, +) => void; +export function DataInput(): ( + target: NgxRootFormComponent, + propertyKey: string, +) => void { return function( - target: NgxRootFormComponent, + target: NgxRootFormComponent | NgxRootFormRemapComponent, propertyKey: string, ) { if (propertyKey !== 'dataInput') { @@ -19,7 +26,9 @@ export function DataInput() { Object.defineProperty(target, propertyKey, { set: function(dataInputValue) { - (this as NgxRootFormComponent).dataInputUpdated(dataInputValue); + (this as + | NgxRootFormComponent + | NgxRootFormRemapComponent).dataInputUpdated(dataInputValue); }, }); }; diff --git a/src/app/main/listing/listing-form/listing-form.component.ts b/src/app/main/listing/listing-form/listing-form.component.ts index 75fecc91..56cac956 100644 --- a/src/app/main/listing/listing-form/listing-form.component.ts +++ b/src/app/main/listing/listing-form/listing-form.component.ts @@ -2,11 +2,10 @@ import { Component, EventEmitter, Input, Output } from '@angular/core'; import { FormControl, Validators } from '@angular/forms'; import { Controls, - takeUntilDestroyed, - // NgxAutomaticRootFormComponent, + // NgxAutomaticRootFormRemapComponent, // NGX_SUB_FORM_HANDLE_VALUE_CHANGES_RATE_STRATEGIES, DataInput, - NgxRootFormComponent, + NgxRootFormRemapComponent, } from 'ngx-sub-form'; import { tap } from 'rxjs/operators'; import { ListingType, OneListing } from 'src/app/interfaces/listing.interface'; @@ -26,7 +25,7 @@ interface OneListingForm { } // if you wish to try the automatic root form component uncomment lines containing: -// - `extends NgxAutomaticRootFormComponent` +// - `extends NgxAutomaticRootFormRemapComponent` // - the `handleDataOutput` method // - the 3 related imports at the top @@ -35,8 +34,8 @@ interface OneListingForm { templateUrl: './listing-form.component.html', styleUrls: ['./listing-form.component.scss'], }) -// export class ListingFormComponent extends NgxAutomaticRootFormComponent -export class ListingFormComponent extends NgxRootFormComponent { +// export class ListingFormComponent extends NgxAutomaticRootFormRemapComponent +export class ListingFormComponent extends NgxRootFormRemapComponent { @DataInput() // tslint:disable-next-line:no-input-rename @Input('listing') diff --git a/src/readme/listing.component.ts b/src/readme/listing.component.ts index ea27cddb..9c23fa65 100644 --- a/src/readme/listing.component.ts +++ b/src/readme/listing.component.ts @@ -1,9 +1,10 @@ import { Component, EventEmitter, Input, Output } from '@angular/core'; import { FormControl, Validators } from '@angular/forms'; import { OneDroid } from '../app/interfaces/droid.interface'; -import { OneListing } from '../app/interfaces/listing.interface'; +import { OneListing, VehicleListing, DroidListing } from '../app/interfaces/listing.interface'; import { OneVehicle } from '../app/interfaces/vehicle.interface'; -import { Controls, DataInput, NgxAutomaticRootFormComponent } from 'ngx-sub-form'; +import { Controls, DataInput, NgxAutomaticRootFormRemapComponent } from 'ngx-sub-form'; +import { UnreachableCase } from '../app/shared/utils'; enum ListingType { VEHICLE = 'Vehicle', @@ -27,9 +28,9 @@ export interface OneListingForm { templateUrl: './listing.component.html', styleUrls: ['./listing.component.scss'], }) -export class ListingComponent extends NgxAutomaticRootFormComponent { +export class ListingComponent extends NgxAutomaticRootFormRemapComponent { // as we're renaming the input, it'd be impossible for ngx-sub-form to guess - // the name of your input to then check within the `ngOnChanges` hook wheter + // the name of your input to then check within the `ngOnChanges` hook whether // it has been updated or not // another solution would be to ask you to use a setter and call a hook but // this is too verbose, that's why we created a decorator `@DataInput` @@ -55,4 +56,49 @@ export class ListingComponent extends NgxAutomaticRootFormComponent | null, + ): OneListingForm | null { + if (!obj) { + return null; + } + + return { + id: obj.id, + title: obj.title, + price: obj.price, + imageUrl: obj.imageUrl, + + listingType: obj.listingType, + vehicleProduct: obj.listingType === ListingType.VEHICLE ? obj.product : null, + droidProduct: obj.listingType === ListingType.DROID ? obj.product : null, + }; + } + + protected transformFromFormGroup(formValue: OneListingForm): VehicleListing | DroidListing | null { + const { id, title, price, imageUrl, listingType } = formValue; + const base = { id, title, price, imageUrl, listingType }; + + switch (formValue.listingType) { + case null: { + throw new Error(`listingType is set but the corresponding value is null`); + } + case ListingType.DROID: { + if (!formValue.droidProduct) { + throw new Error(`listingType is of type DROID but droidProduct is not defined`); + } + return { ...base, listingType: ListingType.DROID, product: formValue.droidProduct }; + } + case ListingType.VEHICLE: { + if (!formValue.droidProduct) { + throw new Error(`listingType is of type VEHICLE but droidProduct is not defined`); + } + return { ...base, listingType: ListingType.DROID, product: formValue.droidProduct }; + } + default: + throw new UnreachableCase(formValue.listingType); + } + } }