From 17eaef0311990e1a06c028f19bc6c63dcd775f31 Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Wed, 13 Sep 2017 14:00:10 -0700 Subject: [PATCH] refactor(forms): Remove readonly getters in forms (#19188) PR Close #19188 --- packages/forms/src/directives/ng_form.ts | 12 +- packages/forms/src/directives/ng_model.ts | 11 +- .../reactive_directives/form_control_name.ts | 7 +- .../form_group_directive.ts | 15 ++- packages/forms/src/model.ts | 110 ++++++++---------- packages/forms/test/directives_spec.ts | 2 +- .../test/value_accessor_integration_spec.ts | 2 +- 7 files changed, 71 insertions(+), 88 deletions(-) diff --git a/packages/forms/src/directives/ng_form.ts b/packages/forms/src/directives/ng_form.ts index a338f62e24..c0b9bfb680 100644 --- a/packages/forms/src/directives/ng_form.ts +++ b/packages/forms/src/directives/ng_form.ts @@ -65,7 +65,8 @@ const resolvedPromise = Promise.resolve(null); }) export class NgForm extends ControlContainer implements Form, AfterViewInit { - private _submitted: boolean = false; + public readonly submitted: boolean = false; + private _directives: NgModel[] = []; form: FormGroup; @@ -97,8 +98,6 @@ export class NgForm extends ControlContainer implements Form, ngAfterViewInit() { this._setUpdateStrategy(); } - get submitted(): boolean { return this._submitted; } - get formDirective(): Form { return this; } get control(): FormGroup { return this.form; } @@ -110,7 +109,8 @@ export class NgForm extends ControlContainer implements Form, addControl(dir: NgModel): void { resolvedPromise.then(() => { const container = this._findContainer(dir.path); - dir._control = container.registerControl(dir.name, dir.control); + (dir as{control: FormControl}).control = + container.registerControl(dir.name, dir.control); setUpControl(dir.control, dir); dir.control.updateValueAndValidity({emitEvent: false}); this._directives.push(dir); @@ -160,7 +160,7 @@ export class NgForm extends ControlContainer implements Form, setValue(value: {[key: string]: any}): void { this.control.setValue(value); } onSubmit($event: Event): boolean { - this._submitted = true; + (this as{submitted: boolean}).submitted = true; syncPendingControls(this.form, this._directives); this.ngSubmit.emit($event); return false; @@ -170,7 +170,7 @@ export class NgForm extends ControlContainer implements Form, resetForm(value: any = undefined): void { this.form.reset(value); - this._submitted = false; + (this as{submitted: boolean}).submitted = false; } private _setUpdateStrategy() { diff --git a/packages/forms/src/directives/ng_model.ts b/packages/forms/src/directives/ng_model.ts index 82764121b8..f98d68b1a8 100644 --- a/packages/forms/src/directives/ng_model.ts +++ b/packages/forms/src/directives/ng_model.ts @@ -110,8 +110,7 @@ const resolvedPromise = Promise.resolve(null); }) export class NgModel extends NgControl implements OnChanges, OnDestroy { - /** @internal */ - _control = new FormControl(); + public readonly control: FormControl = new FormControl(); /** @internal */ _registered = false; viewModel: any; @@ -188,8 +187,6 @@ export class NgModel extends NgControl implements OnChanges, ngOnDestroy(): void { this.formDirective && this.formDirective.removeControl(this); } - get control(): FormControl { return this._control; } - get path(): string[] { return this._parent ? controlPath(this.name, this._parent) : [this.name]; } @@ -216,7 +213,7 @@ export class NgModel extends NgControl implements OnChanges, private _setUpdateStrategy(): void { if (this.options && this.options.updateOn != null) { - this._control._updateOn = this.options.updateOn; + this.control._updateOn = this.options.updateOn; } } @@ -225,8 +222,8 @@ export class NgModel extends NgControl implements OnChanges, } private _setUpStandalone(): void { - setUpControl(this._control, this); - this._control.updateValueAndValidity({emitEvent: false}); + setUpControl(this.control, this); + this.control.updateValueAndValidity({emitEvent: false}); } private _checkForErrors(): void { diff --git a/packages/forms/src/directives/reactive_directives/form_control_name.ts b/packages/forms/src/directives/reactive_directives/form_control_name.ts index 61b89cf863..a53f69b68d 100644 --- a/packages/forms/src/directives/reactive_directives/form_control_name.ts +++ b/packages/forms/src/directives/reactive_directives/form_control_name.ts @@ -82,8 +82,7 @@ export class FormControlName extends NgControl implements OnChanges, OnDestroy { private _added = false; /** @internal */ viewModel: any; - /** @internal */ - _control: FormControl; + readonly control: FormControl; @Input('formControlName') name: string; @@ -135,8 +134,6 @@ export class FormControlName extends NgControl implements OnChanges, OnDestroy { return composeAsyncValidators(this._rawAsyncValidators) !; } - get control(): FormControl { return this._control; } - private _checkParentType(): void { if (!(this._parent instanceof FormGroupName) && this._parent instanceof AbstractFormGroupDirective) { @@ -150,7 +147,7 @@ export class FormControlName extends NgControl implements OnChanges, OnDestroy { private _setUpControl() { this._checkParentType(); - this._control = this.formDirective.addControl(this); + (this as{control: FormControl}).control = this.formDirective.addControl(this); if (this.control.disabled && this.valueAccessor !.setDisabledState) { this.valueAccessor !.setDisabledState !(true); } diff --git a/packages/forms/src/directives/reactive_directives/form_group_directive.ts b/packages/forms/src/directives/reactive_directives/form_group_directive.ts index 40ad30b26c..f172bcab77 100644 --- a/packages/forms/src/directives/reactive_directives/form_group_directive.ts +++ b/packages/forms/src/directives/reactive_directives/form_group_directive.ts @@ -66,7 +66,8 @@ export const formDirectiveProvider: any = { }) export class FormGroupDirective extends ControlContainer implements Form, OnChanges { - private _submitted: boolean = false; + public readonly submitted: boolean = false; + private _oldForm: FormGroup; directives: FormControlName[] = []; @@ -88,8 +89,6 @@ export class FormGroupDirective extends ControlContainer implements Form, } } - get submitted(): boolean { return this._submitted; } - get formDirective(): Form { return this; } get control(): FormGroup { return this.form; } @@ -134,7 +133,7 @@ export class FormGroupDirective extends ControlContainer implements Form, } onSubmit($event: Event): boolean { - this._submitted = true; + (this as{submitted: boolean}).submitted = true; syncPendingControls(this.form, this.directives); this.ngSubmit.emit($event); return false; @@ -144,7 +143,7 @@ export class FormGroupDirective extends ControlContainer implements Form, resetForm(value: any = undefined): void { this.form.reset(value); - this._submitted = false; + (this as{submitted: boolean}).submitted = false; } @@ -152,10 +151,10 @@ export class FormGroupDirective extends ControlContainer implements Form, _updateDomValue() { this.directives.forEach(dir => { const newCtrl: any = this.form.get(dir.path); - if (dir._control !== newCtrl) { - cleanUpControl(dir._control, dir); + if (dir.control !== newCtrl) { + cleanUpControl(dir.control, dir); if (newCtrl) setUpControl(newCtrl, dir); - dir._control = newCtrl; + (dir as{control: FormControl}).control = newCtrl; } }); diff --git a/packages/forms/src/model.ts b/packages/forms/src/model.ts index 7d240d0ce5..c41a3378f0 100644 --- a/packages/forms/src/model.ts +++ b/packages/forms/src/model.ts @@ -106,9 +106,6 @@ function isOptionsObj( * @stable */ export abstract class AbstractControl { - /** @internal */ - _value: any; - /** @internal */ _pendingDirty: boolean; @@ -121,22 +118,12 @@ export abstract class AbstractControl { /** @internal */ _updateOn: FormHooks; - private _valueChanges: EventEmitter; - private _statusChanges: EventEmitter; - private _status: string; - private _errors: ValidationErrors|null; - private _pristine: boolean = true; - private _touched: boolean = false; private _parent: FormGroup|FormArray; private _asyncValidationSubscription: any; + public readonly value: any; constructor(public validator: ValidatorFn|null, public asyncValidator: AsyncValidatorFn|null) {} - /** - * The value of the control. - */ - get value(): any { return this._value; } - /** * The parent control. */ @@ -154,7 +141,7 @@ export abstract class AbstractControl { * These statuses are mutually exclusive, so a control cannot be * both valid AND invalid or invalid AND disabled. */ - get status(): string { return this._status; } + public readonly status: string; /** * A control is `valid` when its `status === VALID`. @@ -162,7 +149,7 @@ export abstract class AbstractControl { * In order to have this status, the control must have passed all its * validation checks. */ - get valid(): boolean { return this._status === VALID; } + get valid(): boolean { return this.status === VALID; } /** * A control is `invalid` when its `status === INVALID`. @@ -170,7 +157,7 @@ export abstract class AbstractControl { * In order to have this status, the control must have failed * at least one of its validation checks. */ - get invalid(): boolean { return this._status === INVALID; } + get invalid(): boolean { return this.status === INVALID; } /** * A control is `pending` when its `status === PENDING`. @@ -178,7 +165,7 @@ export abstract class AbstractControl { * In order to have this status, the control must be in the * middle of conducting a validation check. */ - get pending(): boolean { return this._status == PENDING; } + get pending(): boolean { return this.status == PENDING; } /** * A control is `disabled` when its `status === DISABLED`. @@ -187,7 +174,7 @@ export abstract class AbstractControl { * are not included in the aggregate value of their ancestor * controls. */ - get disabled(): boolean { return this._status === DISABLED; } + get disabled(): boolean { return this.status === DISABLED; } /** * A control is `enabled` as long as its `status !== DISABLED`. @@ -195,13 +182,13 @@ export abstract class AbstractControl { * In other words, it has a status of `VALID`, `INVALID`, or * `PENDING`. */ - get enabled(): boolean { return this._status !== DISABLED; } + get enabled(): boolean { return this.status !== DISABLED; } /** * Returns any errors generated by failing validation. If there * are no errors, it will return null. */ - get errors(): ValidationErrors|null { return this._errors; } + public readonly errors: ValidationErrors|null; /** * A control is `pristine` if the user has not yet changed @@ -210,7 +197,7 @@ export abstract class AbstractControl { * Note that programmatic changes to a control's value will * *not* mark it dirty. */ - get pristine(): boolean { return this._pristine; } + public readonly pristine: boolean = true; /** * A control is `dirty` if the user has changed the value @@ -225,25 +212,25 @@ export abstract class AbstractControl { * A control is marked `touched` once the user has triggered * a `blur` event on it. */ - get touched(): boolean { return this._touched; } + public readonly touched: boolean = false; /** * A control is `untouched` if the user has not yet triggered * a `blur` event on it. */ - get untouched(): boolean { return !this._touched; } + get untouched(): boolean { return !this.touched; } /** * Emits an event every time the value of the control changes, in * the UI or programmatically. */ - get valueChanges(): Observable { return this._valueChanges; } + public readonly valueChanges: Observable; /** * Emits an event every time the validation status of the control * is re-calculated. */ - get statusChanges(): Observable { return this._statusChanges; } + public readonly statusChanges: Observable; /** * Returns the update strategy of the `AbstractControl` (i.e. @@ -287,7 +274,7 @@ export abstract class AbstractControl { * the model. */ markAsTouched(opts: {onlySelf?: boolean} = {}): void { - this._touched = true; + (this as{touched: boolean}).touched = true; if (this._parent && !opts.onlySelf) { this._parent.markAsTouched(opts); @@ -302,7 +289,7 @@ export abstract class AbstractControl { * controls. */ markAsUntouched(opts: {onlySelf?: boolean} = {}): void { - this._touched = false; + (this as{touched: boolean}).touched = false; this._pendingTouched = false; this._forEachChild( @@ -320,7 +307,7 @@ export abstract class AbstractControl { * the model. */ markAsDirty(opts: {onlySelf?: boolean} = {}): void { - this._pristine = false; + (this as{pristine: boolean}).pristine = false; if (this._parent && !opts.onlySelf) { this._parent.markAsDirty(opts); @@ -335,7 +322,7 @@ export abstract class AbstractControl { * controls. */ markAsPristine(opts: {onlySelf?: boolean} = {}): void { - this._pristine = true; + (this as{pristine: boolean}).pristine = true; this._pendingDirty = false; this._forEachChild((control: AbstractControl) => { control.markAsPristine({onlySelf: true}); }); @@ -349,7 +336,7 @@ export abstract class AbstractControl { * Marks the control as `pending`. */ markAsPending(opts: {onlySelf?: boolean} = {}): void { - this._status = PENDING; + (this as{status: string}).status = PENDING; if (this._parent && !opts.onlySelf) { this._parent.markAsPending(opts); @@ -363,14 +350,14 @@ export abstract class AbstractControl { * If the control has children, all children will be disabled to maintain the model. */ disable(opts: {onlySelf?: boolean, emitEvent?: boolean} = {}): void { - this._status = DISABLED; - this._errors = null; + (this as{status: string}).status = DISABLED; + (this as{errors: ValidationErrors | null}).errors = null; this._forEachChild((control: AbstractControl) => { control.disable({onlySelf: true}); }); this._updateValue(); if (opts.emitEvent !== false) { - this._valueChanges.emit(this._value); - this._statusChanges.emit(this._status); + (this.valueChanges as EventEmitter).emit(this.value); + (this.statusChanges as EventEmitter).emit(this.status); } this._updateAncestors(!!opts.onlySelf); @@ -385,7 +372,7 @@ export abstract class AbstractControl { * If the control has children, all children will be enabled. */ enable(opts: {onlySelf?: boolean, emitEvent?: boolean} = {}): void { - this._status = VALID; + (this as{status: string}).status = VALID; this._forEachChild((control: AbstractControl) => { control.enable({onlySelf: true}); }); this.updateValueAndValidity({onlySelf: true, emitEvent: opts.emitEvent}); @@ -429,17 +416,17 @@ export abstract class AbstractControl { if (this.enabled) { this._cancelExistingSubscription(); - this._errors = this._runValidator(); - this._status = this._calculateStatus(); + (this as{errors: ValidationErrors | null}).errors = this._runValidator(); + (this as{status: string}).status = this._calculateStatus(); - if (this._status === VALID || this._status === PENDING) { + if (this.status === VALID || this.status === PENDING) { this._runAsyncValidator(opts.emitEvent); } } if (opts.emitEvent !== false) { - this._valueChanges.emit(this._value); - this._statusChanges.emit(this._status); + (this.valueChanges as EventEmitter).emit(this.value); + (this.statusChanges as EventEmitter).emit(this.status); } if (this._parent && !opts.onlySelf) { @@ -453,7 +440,9 @@ export abstract class AbstractControl { this.updateValueAndValidity({onlySelf: true, emitEvent: opts.emitEvent}); } - private _setInitialStatus() { this._status = this._allControlsDisabled() ? DISABLED : VALID; } + private _setInitialStatus() { + (this as{status: string}).status = this._allControlsDisabled() ? DISABLED : VALID; + } private _runValidator(): ValidationErrors|null { return this.validator ? this.validator(this) : null; @@ -461,7 +450,7 @@ export abstract class AbstractControl { private _runAsyncValidator(emitEvent?: boolean): void { if (this.asyncValidator) { - this._status = PENDING; + (this as{status: string}).status = PENDING; const obs = toObservable(this.asyncValidator(this)); this._asyncValidationSubscription = obs.subscribe((errors: ValidationErrors | null) => this.setErrors(errors, {emitEvent})); @@ -498,7 +487,7 @@ export abstract class AbstractControl { * ``` */ setErrors(errors: ValidationErrors|null, opts: {emitEvent?: boolean} = {}): void { - this._errors = errors; + (this as{errors: ValidationErrors | null}).errors = errors; this._updateControlsErrors(opts.emitEvent !== false); } @@ -525,7 +514,7 @@ export abstract class AbstractControl { */ getError(errorCode: string, path?: string[]): any { const control = path ? this.get(path) : this; - return control && control._errors ? control._errors[errorCode] : null; + return control && control.errors ? control.errors[errorCode] : null; } /** @@ -551,10 +540,10 @@ export abstract class AbstractControl { /** @internal */ _updateControlsErrors(emitEvent: boolean): void { - this._status = this._calculateStatus(); + (this as{status: string}).status = this._calculateStatus(); if (emitEvent) { - this._statusChanges.emit(this._status); + (this.statusChanges as EventEmitter).emit(this.status); } if (this._parent) { @@ -564,14 +553,14 @@ export abstract class AbstractControl { /** @internal */ _initObservables() { - this._valueChanges = new EventEmitter(); - this._statusChanges = new EventEmitter(); + (this as{valueChanges: Observable}).valueChanges = new EventEmitter(); + (this as{statusChanges: Observable}).statusChanges = new EventEmitter(); } private _calculateStatus(): string { if (this._allControlsDisabled()) return DISABLED; - if (this._errors) return INVALID; + if (this.errors) return INVALID; if (this._anyControlsHaveStatus(PENDING)) return PENDING; if (this._anyControlsHaveStatus(INVALID)) return INVALID; return VALID; @@ -609,7 +598,7 @@ export abstract class AbstractControl { /** @internal */ _updatePristine(opts: {onlySelf?: boolean} = {}): void { - this._pristine = !this._anyControlsDirty(); + (this as{pristine: boolean}).pristine = !this._anyControlsDirty(); if (this._parent && !opts.onlySelf) { this._parent._updatePristine(opts); @@ -618,7 +607,7 @@ export abstract class AbstractControl { /** @internal */ _updateTouched(opts: {onlySelf?: boolean} = {}): void { - this._touched = this._anyControlsTouched(); + (this as{touched: boolean}).touched = this._anyControlsTouched(); if (this._parent && !opts.onlySelf) { this._parent._updateTouched(opts); @@ -755,10 +744,10 @@ export class FormControl extends AbstractControl { emitModelToViewChange?: boolean, emitViewToModelChange?: boolean } = {}): void { - this._value = this._pendingValue = value; + (this as{value: any}).value = this._pendingValue = value; if (this._onChange.length && options.emitModelToViewChange !== false) { this._onChange.forEach( - (changeFn) => changeFn(this._value, options.emitViewToModelChange !== false)); + (changeFn) => changeFn(this.value, options.emitViewToModelChange !== false)); } this.updateValueAndValidity(options); } @@ -811,7 +800,7 @@ export class FormControl extends AbstractControl { this._applyFormState(formState); this.markAsPristine(options); this.markAsUntouched(options); - this.setValue(this._value, options); + this.setValue(this.value, options); } /** @@ -868,11 +857,11 @@ export class FormControl extends AbstractControl { private _applyFormState(formState: any) { if (this._isBoxedValue(formState)) { - this._value = this._pendingValue = formState.value; + (this as{value: any}).value = this._pendingValue = formState.value; formState.disabled ? this.disable({onlySelf: true, emitEvent: false}) : this.enable({onlySelf: true, emitEvent: false}); } else { - this._value = this._pendingValue = formState; + (this as{value: any}).value = this._pendingValue = formState; } } } @@ -1172,7 +1161,7 @@ export class FormGroup extends AbstractControl { } /** @internal */ - _updateValue(): void { this._value = this._reduceValue(); } + _updateValue(): void { (this as{value: any}).value = this._reduceValue(); } /** @internal */ _anyControls(condition: Function): boolean { @@ -1498,8 +1487,9 @@ export class FormArray extends AbstractControl { /** @internal */ _updateValue(): void { - this._value = this.controls.filter((control) => control.enabled || this.disabled) - .map((control) => control.value); + (this as{value: any}).value = + this.controls.filter((control) => control.enabled || this.disabled) + .map((control) => control.value); } /** @internal */ diff --git a/packages/forms/test/directives_spec.ts b/packages/forms/test/directives_spec.ts index 02c4363c5a..63ab7650b1 100644 --- a/packages/forms/test/directives_spec.ts +++ b/packages/forms/test/directives_spec.ts @@ -650,7 +650,7 @@ export function main() { parent.form = new FormGroup({'name': formModel}); controlNameDir = new FormControlName(parent, [], [], [defaultAccessor]); controlNameDir.name = 'name'; - controlNameDir._control = formModel; + (controlNameDir as{control: FormControl}).control = formModel; }); it('should reexport control properties', () => { diff --git a/packages/forms/test/value_accessor_integration_spec.ts b/packages/forms/test/value_accessor_integration_spec.ts index 11ad0271d6..77030cebb2 100644 --- a/packages/forms/test/value_accessor_integration_spec.ts +++ b/packages/forms/test/value_accessor_integration_spec.ts @@ -1397,4 +1397,4 @@ export class NgModelCustomComp implements ControlValueAccessor { export class NgModelCustomWrapper { name: string; isDisabled = false; -} \ No newline at end of file +}