From 34feecf60eb4b9f94d4c9887e38f76a252d9ce40 Mon Sep 17 00:00:00 2001 From: Kara Date: Wed, 13 Jul 2016 14:13:02 -0700 Subject: [PATCH] fix(forms): improve no value accessor error message (#10051) --- .../src/forms-deprecated/directives/shared.ts | 23 +++++++----- .../test/forms-deprecated/directives_spec.ts | 25 +++++++++++-- .../@angular/forms/src/directives/ng_model.ts | 2 +- .../@angular/forms/src/directives/shared.ts | 23 +++++++----- .../@angular/forms/test/directives_spec.ts | 35 ++++++++++++++++--- 5 files changed, 85 insertions(+), 23 deletions(-) diff --git a/modules/@angular/common/src/forms-deprecated/directives/shared.ts b/modules/@angular/common/src/forms-deprecated/directives/shared.ts index 3525168348..499b56453c 100644 --- a/modules/@angular/common/src/forms-deprecated/directives/shared.ts +++ b/modules/@angular/common/src/forms-deprecated/directives/shared.ts @@ -34,8 +34,8 @@ export function controlPath(name: string, parent: ControlContainer): string[] { } export function setUpControl(control: Control, dir: NgControl): void { - if (isBlank(control)) _throwError(dir, 'Cannot find control'); - if (isBlank(dir.valueAccessor)) _throwError(dir, 'No value accessor for'); + if (isBlank(control)) _throwError(dir, 'Cannot find control with'); + if (isBlank(dir.valueAccessor)) _throwError(dir, 'No value accessor for form control with'); control.validator = Validators.compose([control.validator, dir.validator]); control.asyncValidator = Validators.composeAsync([control.asyncValidator, dir.asyncValidator]); @@ -56,14 +56,21 @@ export function setUpControl(control: Control, dir: NgControl): void { } export function setUpControlGroup(control: ControlGroup, dir: NgControlGroup) { - if (isBlank(control)) _throwError(dir, 'Cannot find control'); + if (isBlank(control)) _throwError(dir, 'Cannot find control with'); control.validator = Validators.compose([control.validator, dir.validator]); control.asyncValidator = Validators.composeAsync([control.asyncValidator, dir.asyncValidator]); } function _throwError(dir: AbstractControlDirective, message: string): void { - var path = dir.path.join(' -> '); - throw new BaseException(`${message} '${path}'`); + let messageEnd: string; + if (dir.path.length > 1) { + messageEnd = `path: '${dir.path.join(' -> ')}'`; + } else if (dir.path[0]) { + messageEnd = `name: '${dir.path}'`; + } else { + messageEnd = 'unspecified name'; + } + throw new BaseException(`${message} ${messageEnd}`); } export function composeValidators(validators: /* Array */ any[]): ValidatorFn { @@ -102,12 +109,12 @@ export function selectValueAccessor( hasConstructor(v, SelectMultipleControlValueAccessor) || hasConstructor(v, RadioControlValueAccessor)) { if (isPresent(builtinAccessor)) - _throwError(dir, 'More than one built-in value accessor matches'); + _throwError(dir, 'More than one built-in value accessor matches form control with'); builtinAccessor = v; } else { if (isPresent(customAccessor)) - _throwError(dir, 'More than one custom value accessor matches'); + _throwError(dir, 'More than one custom value accessor matches form control with'); customAccessor = v; } }); @@ -116,6 +123,6 @@ export function selectValueAccessor( if (isPresent(builtinAccessor)) return builtinAccessor; if (isPresent(defaultAccessor)) return defaultAccessor; - _throwError(dir, 'No valid value accessor for'); + _throwError(dir, 'No valid value accessor for form control with'); return null; } diff --git a/modules/@angular/common/test/forms-deprecated/directives_spec.ts b/modules/@angular/common/test/forms-deprecated/directives_spec.ts index 64ac6b934b..31dc054f21 100644 --- a/modules/@angular/common/test/forms-deprecated/directives_spec.ts +++ b/modules/@angular/common/test/forms-deprecated/directives_spec.ts @@ -149,14 +149,27 @@ export function main() { var dir = new NgControlName(form, null, null, [defaultAccessor]); dir.name = 'invalidName'; - expect(() => form.addControl(dir)).toThrowError(/Cannot find control 'invalidName'/); + expect(() => form.addControl(dir)) + .toThrowError(new RegExp(`Cannot find control with name: 'invalidName'`)); }); it('should throw when no value accessor', () => { var dir = new NgControlName(form, null, null, null); dir.name = 'login'; - expect(() => form.addControl(dir)).toThrowError(/No value accessor for 'login'/); + expect(() => form.addControl(dir)) + .toThrowError(new RegExp(`No value accessor for form control with name: 'login'`)); + }); + + it('should throw when no value accessor with path', () => { + const group = new NgControlGroup(form, null, null); + const dir = new NgControlName(group, null, null, null); + group.name = 'passwords'; + dir.name = 'password'; + + expect(() => form.addControl(dir)) + .toThrowError(new RegExp( + `No value accessor for form control with path: 'passwords -> password'`)); }); it('should set up validators', fakeAsync(() => { @@ -432,6 +445,14 @@ export function main() { expect(ngModel.untouched).toBe(control.untouched); }); + it('should throw when no value accessor with unnamed control', () => { + const unnamedDir = new NgModel(null, null, null); + + expect(() => unnamedDir.ngOnChanges({})) + .toThrowError(new RegExp(`No value accessor for form control with unspecified name`)); + }); + + it('should set up validator', fakeAsync(() => { // this will add the required validator and recalculate the validity ngModel.ngOnChanges({}); diff --git a/modules/@angular/forms/src/directives/ng_model.ts b/modules/@angular/forms/src/directives/ng_model.ts index 64df9d83b8..7e07820e1d 100644 --- a/modules/@angular/forms/src/directives/ng_model.ts +++ b/modules/@angular/forms/src/directives/ng_model.ts @@ -89,7 +89,7 @@ export class NgModel extends NgControl implements OnChanges, get control(): FormControl { return this._control; } get path(): string[] { - return this._parent ? controlPath(this.name, this._parent) : []; + return this._parent ? controlPath(this.name, this._parent) : [this.name]; } get formDirective(): any { return this._parent ? this._parent.formDirective : null; } diff --git a/modules/@angular/forms/src/directives/shared.ts b/modules/@angular/forms/src/directives/shared.ts index 3cd4c8046d..16a8c2a323 100644 --- a/modules/@angular/forms/src/directives/shared.ts +++ b/modules/@angular/forms/src/directives/shared.ts @@ -35,8 +35,8 @@ export function controlPath(name: string, parent: ControlContainer): string[] { } export function setUpControl(control: FormControl, dir: NgControl): void { - if (isBlank(control)) _throwError(dir, 'Cannot find control'); - if (isBlank(dir.valueAccessor)) _throwError(dir, 'No value accessor for'); + if (isBlank(control)) _throwError(dir, 'Cannot find control with'); + if (isBlank(dir.valueAccessor)) _throwError(dir, 'No value accessor for form control with'); control.validator = Validators.compose([control.validator, dir.validator]); control.asyncValidator = Validators.composeAsync([control.asyncValidator, dir.asyncValidator]); @@ -63,14 +63,21 @@ export function setUpControl(control: FormControl, dir: NgControl): void { export function setUpFormContainer( control: FormGroup | FormArray, dir: AbstractFormGroupDirective | FormArrayName) { - if (isBlank(control)) _throwError(dir, 'Cannot find control'); + if (isBlank(control)) _throwError(dir, 'Cannot find control with'); control.validator = Validators.compose([control.validator, dir.validator]); control.asyncValidator = Validators.composeAsync([control.asyncValidator, dir.asyncValidator]); } function _throwError(dir: AbstractControlDirective, message: string): void { - var path = dir.path.join(' -> '); - throw new BaseException(`${message} '${path}'`); + let messageEnd: string; + if (dir.path.length > 1) { + messageEnd = `path: '${dir.path.join(' -> ')}'`; + } else if (dir.path[0]) { + messageEnd = `name: '${dir.path}'`; + } else { + messageEnd = 'unspecified name attribute'; + } + throw new BaseException(`${message} ${messageEnd}`); } export function composeValidators(validators: /* Array */ any[]): ValidatorFn { @@ -109,12 +116,12 @@ export function selectValueAccessor( hasConstructor(v, SelectMultipleControlValueAccessor) || hasConstructor(v, RadioControlValueAccessor)) { if (isPresent(builtinAccessor)) - _throwError(dir, 'More than one built-in value accessor matches'); + _throwError(dir, 'More than one built-in value accessor matches form control with'); builtinAccessor = v; } else { if (isPresent(customAccessor)) - _throwError(dir, 'More than one custom value accessor matches'); + _throwError(dir, 'More than one custom value accessor matches form control with'); customAccessor = v; } }); @@ -123,6 +130,6 @@ export function selectValueAccessor( if (isPresent(builtinAccessor)) return builtinAccessor; if (isPresent(defaultAccessor)) return defaultAccessor; - _throwError(dir, 'No valid value accessor for'); + _throwError(dir, 'No valid value accessor for form control with'); return null; } diff --git a/modules/@angular/forms/test/directives_spec.ts b/modules/@angular/forms/test/directives_spec.ts index 1d4dc8edf1..e2ce55c628 100644 --- a/modules/@angular/forms/test/directives_spec.ts +++ b/modules/@angular/forms/test/directives_spec.ts @@ -166,15 +166,26 @@ export function main() { dir.name = 'invalidName'; expect(() => form.addControl(dir)) - .toThrowError(new RegExp('Cannot find control \'invalidName\'')); + .toThrowError(new RegExp(`Cannot find control with name: 'invalidName'`)); }); - it('should throw when no value accessor', () => { - var dir = new FormControlName(form, null, null, null); + it('should throw for a named control when no value accessor', () => { + const dir = new FormControlName(form, null, null, null); dir.name = 'login'; expect(() => form.addControl(dir)) - .toThrowError(new RegExp('No value accessor for \'login\'')); + .toThrowError(new RegExp(`No value accessor for form control with name: 'login'`)); + }); + + it('should throw when no value accessor with path', () => { + const group = new FormGroupName(form, null, null); + const dir = new FormControlName(group, null, null, null); + group.name = 'passwords'; + dir.name = 'password'; + + expect(() => form.addControl(dir)) + .toThrowError(new RegExp( + `No value accessor for form control with path: 'passwords -> password'`)); }); it('should set up validators', fakeAsync(() => { @@ -482,6 +493,22 @@ export function main() { expect(ngModel.valueChanges).toBe(control.valueChanges); }); + it('should throw when no value accessor with named control', () => { + const namedDir = new NgModel(null, null, null, null); + namedDir.name = 'one'; + + expect(() => namedDir.ngOnChanges({})) + .toThrowError(new RegExp(`No value accessor for form control with name: 'one'`)); + }); + + it('should throw when no value accessor with unnamed control', () => { + const unnamedDir = new NgModel(null, null, null, null); + + expect(() => unnamedDir.ngOnChanges({})) + .toThrowError( + new RegExp(`No value accessor for form control with unspecified name attribute`)); + }); + it('should set up validator', fakeAsync(() => { // this will add the required validator and recalculate the validity ngModel.ngOnChanges({});