From 42cfa3c17087f655a1f692385ad49c32a903a1be Mon Sep 17 00:00:00 2001 From: vsavkin Date: Tue, 3 Nov 2015 12:56:34 -0800 Subject: [PATCH] feat(forms): remove controlsErrors BREAKING CHANGE Previously, the controlsErrors getter of ControlGroup and ControlArray returned the errors of their direct children. This was confusing because the result did not include the errors of nested children (ControlGroup -> ControlGroup -> Control). Making controlsErrors to include such errors would require inventing some custom serialization format, which applications would have to understand. Since controlsErrors was just a convenience method, and it was causing confusing, we are removing it. If you want to get the errors of the whole form serialized into a single object, you can manually traverse the form and accumulate the errors. This way you have more control over how the errors are serialized. --- .../directives/abstract_control_directive.ts | 2 - modules/angular2/src/common/forms/model.ts | 40 +---------- .../angular2/test/common/forms/model_spec.ts | 69 ------------------- modules/angular2/test/public_api_spec.ts | 13 ---- 4 files changed, 1 insertion(+), 123 deletions(-) diff --git a/modules/angular2/src/common/forms/directives/abstract_control_directive.ts b/modules/angular2/src/common/forms/directives/abstract_control_directive.ts index 85f898fef9d1..989fb80162e7 100644 --- a/modules/angular2/src/common/forms/directives/abstract_control_directive.ts +++ b/modules/angular2/src/common/forms/directives/abstract_control_directive.ts @@ -18,8 +18,6 @@ export abstract class AbstractControlDirective { return isPresent(this.control) ? this.control.errors : null; } - get controlsErrors(): any { return isPresent(this.control) ? this.control.controlsErrors : null; } - get pristine(): boolean { return isPresent(this.control) ? this.control.pristine : null; } get dirty(): boolean { return isPresent(this.control) ? this.control.dirty : null; } diff --git a/modules/angular2/src/common/forms/model.ts b/modules/angular2/src/common/forms/model.ts index 0a3fe951a024..8d91dcd46cfa 100644 --- a/modules/angular2/src/common/forms/model.ts +++ b/modules/angular2/src/common/forms/model.ts @@ -58,7 +58,6 @@ export abstract class AbstractControl { private _statusChanges: EventEmitter; private _status: string; private _errors: {[key: string]: any}; - private _controlsErrors: any; private _pristine: boolean = true; private _touched: boolean = false; private _parent: ControlGroup | ControlArray; @@ -77,11 +76,6 @@ export abstract class AbstractControl { */ get errors(): {[key: string]: any} { return this._errors; } - /** - * Returns the errors of the child controls. - */ - get controlsErrors(): any { return this._controlsErrors; } - get pristine(): boolean { return this._pristine; } get dirty(): boolean { return !this.pristine; } @@ -126,7 +120,6 @@ export abstract class AbstractControl { this._updateValue(); this._errors = this._runValidator(); - this._controlsErrors = this._calculateControlsErrors(); this._status = this._calculateStatus(); if (this._status == VALID || this._status == PENDING) { @@ -216,7 +209,6 @@ export abstract class AbstractControl { /** @internal */ _updateControlsErrors(): void { - this._controlsErrors = this._calculateControlsErrors(); this._status = this._calculateStatus(); if (isPresent(this._parent)) { @@ -240,8 +232,7 @@ export abstract class AbstractControl { /** @internal */ abstract _updateValue(): void; - /** @internal */ - abstract _calculateControlsErrors(): any; + /** @internal */ abstract _anyControlsHaveStatus(status: string): boolean; } @@ -301,11 +292,6 @@ export class Control extends AbstractControl { */ _updateValue() {} - /** - * @internal - */ - _calculateControlsErrors() { return null; } - /** * @internal */ @@ -388,17 +374,6 @@ export class ControlGroup extends AbstractControl { /** @internal */ _updateValue() { this._value = this._reduceValue(); } - /** @internal */ - _calculateControlsErrors() { - var res = {}; - StringMapWrapper.forEach(this.controls, (control, name) => { - if (this.contains(name) && isPresent(control.errors)) { - res[name] = control.errors; - } - }); - return StringMapWrapper.isEmpty(res) ? null : res; - } - /** @internal */ _anyControlsHaveStatus(status: string): boolean { var res = false; @@ -503,19 +478,6 @@ export class ControlArray extends AbstractControl { /** @internal */ _updateValue(): void { this._value = this.controls.map((control) => control.value); } - /** @internal */ - _calculateControlsErrors() { - var res = []; - var anyErrors = false; - this.controls.forEach((control) => { - res.push(control.errors); - if (isPresent(control.errors)) { - anyErrors = true; - } - }); - return anyErrors ? res : null; - } - /** @internal */ _anyControlsHaveStatus(status: string): boolean { return ListWrapper.any(this.controls, c => c.status == status); diff --git a/modules/angular2/test/common/forms/model_spec.ts b/modules/angular2/test/common/forms/model_spec.ts index 3537d17f5b4e..eddf9a7a69b7 100644 --- a/modules/angular2/test/common/forms/model_spec.ts +++ b/modules/angular2/test/common/forms/model_spec.ts @@ -302,7 +302,6 @@ export function main() { c.setErrors({"someError": true}); - expect(g.controlsErrors).toEqual({"one": {"someError": true}}); expect(g.valid).toEqual(false); }); @@ -354,41 +353,6 @@ export function main() { }); }); - describe("controlsErrors", () => { - it("should be null when no errors", () => { - var g = new ControlGroup({"one": new Control('value', Validators.required)}); - - expect(g.valid).toEqual(true); - expect(g.controlsErrors).toEqual(null); - }); - - it("should collect errors from the child controls", () => { - var one = new Control(null, Validators.required); - var g = new ControlGroup({"one": one}); - - expect(g.valid).toEqual(false); - expect(g.controlsErrors).toEqual({"one": {"required": true}}); - }); - - it("should not include controls that have no errors", () => { - var one = new Control(null, Validators.required); - var two = new Control("two"); - var g = new ControlGroup({"one": one, "two": two}); - - expect(g.controlsErrors).toEqual({"one": {"required": true}}); - }); - - it("should run the validator with the value changes", () => { - var c = new Control(null, Validators.required); - var g = new ControlGroup({"one": c}); - - c.updateValue("some value"); - - expect(g.valid).toEqual(true); - expect(g.controlsErrors).toEqual(null); - }); - }); - describe("errors", () => { it("should run the validator when the value changes", () => { var simpleValidator = (c) => @@ -667,39 +631,6 @@ export function main() { }); }); - describe("controlsErrors", () => { - it("should return null when no errors", () => { - var a = new ControlArray( - [new Control(1, Validators.required), new Control(2, Validators.required)]); - - expect(a.valid).toBe(true); - expect(a.controlsErrors).toBe(null); - }); - - it("should collect errors from the child controls", () => { - var a = new ControlArray([ - new Control(1, Validators.required), - new Control(null, Validators.required), - new Control(2, Validators.required) - ]); - - expect(a.valid).toBe(false); - expect(a.controlsErrors).toEqual([null, {"required": true}, null]); - }); - - it("should run the validator when the value changes", () => { - var a = new ControlArray([]); - var c = new Control(null, Validators.required); - a.push(c); - expect(a.valid).toBe(false); - - c.updateValue("some value"); - - expect(a.valid).toBe(true); - expect(a.controlsErrors).toBe(null); - }); - }); - describe("errors", () => { it("should run the validator when the value changes", () => { var simpleValidator = (c) => c.controls[0].value != "correct" ? {"broken": true} : null; diff --git a/modules/angular2/test/public_api_spec.ts b/modules/angular2/test/public_api_spec.ts index 120985110f20..f4cae4eda2ac 100644 --- a/modules/angular2/test/public_api_spec.ts +++ b/modules/angular2/test/public_api_spec.ts @@ -43,7 +43,6 @@ var NG_ALL = [ 'AbstractControl', 'AbstractControl.dirty', 'AbstractControl.errors', - 'AbstractControl.controlsErrors', 'AbstractControl.find()', 'AbstractControl.getError()', 'AbstractControl.hasError()', @@ -70,7 +69,6 @@ var NG_ALL = [ 'AbstractControlDirective.control', 'AbstractControlDirective.dirty', 'AbstractControlDirective.errors', - 'AbstractControlDirective.controlsErrors', 'AbstractControlDirective.pristine', 'AbstractControlDirective.touched', 'AbstractControlDirective.untouched', @@ -285,7 +283,6 @@ var NG_ALL = [ 'Control', 'Control.dirty', 'Control.errors', - 'Control.controlsErrors', 'Control.find()', 'Control.getError()', 'Control.hasError()', @@ -316,7 +313,6 @@ var NG_ALL = [ 'ControlArray.controls=', 'ControlArray.dirty', 'ControlArray.errors', - 'ControlArray.controlsErrors', 'ControlArray.find()', 'ControlArray.getError()', 'ControlArray.hasError()', @@ -347,7 +343,6 @@ var NG_ALL = [ 'ControlContainer.control', 'ControlContainer.dirty', 'ControlContainer.errors', - 'ControlContainer.controlsErrors', 'ControlContainer.formDirective', 'ControlContainer.name', 'ControlContainer.name=', @@ -364,7 +359,6 @@ var NG_ALL = [ 'ControlGroup.controls=', 'ControlGroup.dirty', 'ControlGroup.errors', - 'ControlGroup.controlsErrors', 'ControlGroup.exclude()', 'ControlGroup.find()', 'ControlGroup.getError()', @@ -836,7 +830,6 @@ var NG_ALL = [ 'NgControl.control', 'NgControl.dirty', 'NgControl.errors', - 'NgControl.controlsErrors', 'NgControl.name', 'NgControl.name=', 'NgControl.path', @@ -853,7 +846,6 @@ var NG_ALL = [ 'NgControlGroup.control', 'NgControlGroup.dirty', 'NgControlGroup.errors', - 'NgControlGroup.controlsErrors', 'NgControlGroup.formDirective', 'NgControlGroup.name', 'NgControlGroup.name=', @@ -878,7 +870,6 @@ var NG_ALL = [ 'NgControlName.control', 'NgControlName.dirty', 'NgControlName.errors', - 'NgControlName.controlsErrors', 'NgControlName.formDirective', 'NgControlName.model', 'NgControlName.model=', @@ -912,7 +903,6 @@ var NG_ALL = [ 'NgForm.controls', 'NgForm.dirty', 'NgForm.errors', - 'NgForm.controlsErrors', 'NgForm.form', 'NgForm.form=', 'NgForm.formDirective', @@ -936,7 +926,6 @@ var NG_ALL = [ 'NgFormControl.control', 'NgFormControl.dirty', 'NgFormControl.errors', - 'NgFormControl.controlsErrors', 'NgFormControl.form', 'NgFormControl.form=', 'NgFormControl.model', @@ -967,7 +956,6 @@ var NG_ALL = [ 'NgFormModel.directives=', 'NgFormModel.dirty', 'NgFormModel.errors', - 'NgFormModel.controlsErrors', 'NgFormModel.form', 'NgFormModel.form=', 'NgFormModel.formDirective', @@ -994,7 +982,6 @@ var NG_ALL = [ 'NgModel.control', 'NgModel.dirty', 'NgModel.errors', - 'NgModel.controlsErrors', 'NgModel.model', 'NgModel.model=', 'NgModel.name',