From 0aba42ae5b2d8922dc5e6b9fca3cc249b47a0c14 Mon Sep 17 00:00:00 2001 From: Kara Date: Tue, 26 Jul 2016 10:08:46 -0700 Subject: [PATCH] fix(forms): throw error if wrong control container for reactive forms (#10286) --- .../forms-deprecated/directives/ng_form.ts | 2 +- .../directives/ng_form_model.ts | 2 +- .../abstract_form_group_directive.ts | 8 +- .../reactive_directives/form_array_name.ts | 36 +++- .../reactive_directives/form_control_name.ts | 34 +++- .../reactive_directives/form_group_name.ts | 31 +++- .../forms/test/reactive_integration_spec.ts | 165 ++++++++++++++---- 7 files changed, 238 insertions(+), 40 deletions(-) diff --git a/modules/@angular/common/src/forms-deprecated/directives/ng_form.ts b/modules/@angular/common/src/forms-deprecated/directives/ng_form.ts index 1ce5c54759..20568df028 100644 --- a/modules/@angular/common/src/forms-deprecated/directives/ng_form.ts +++ b/modules/@angular/common/src/forms-deprecated/directives/ng_form.ts @@ -118,7 +118,7 @@ export class NgForm extends ControlContainer implements Form { console.warn(` *It looks like you're using the old forms module. This will be opt-in in the next RC, and will eventually be removed in favor of the new forms module. For more information, see: - https://docs.google.com/document/u/1/d/1RIezQqE4aEhBRmArIAS1mRIZtWFf6JxN_7B4meyWK0Y/pub + https://docs.google.com/document/d/1RIezQqE4aEhBRmArIAS1mRIZtWFf6JxN_7B4meyWK0Y/preview `); } } diff --git a/modules/@angular/common/src/forms-deprecated/directives/ng_form_model.ts b/modules/@angular/common/src/forms-deprecated/directives/ng_form_model.ts index b11947809e..8f2c37297c 100644 --- a/modules/@angular/common/src/forms-deprecated/directives/ng_form_model.ts +++ b/modules/@angular/common/src/forms-deprecated/directives/ng_form_model.ts @@ -134,7 +134,7 @@ export class NgFormModel extends ControlContainer implements Form, console.warn(` *It looks like you're using the old forms module. This will be opt-in in the next RC, and will eventually be removed in favor of the new forms module. For more information, see: - https://docs.google.com/document/u/1/d/1RIezQqE4aEhBRmArIAS1mRIZtWFf6JxN_7B4meyWK0Y/pub + https://docs.google.com/document/d/1RIezQqE4aEhBRmArIAS1mRIZtWFf6JxN_7B4meyWK0Y/preview `); } } diff --git a/modules/@angular/forms/src/directives/abstract_form_group_directive.ts b/modules/@angular/forms/src/directives/abstract_form_group_directive.ts index ff1734729b..ab5dedeac2 100644 --- a/modules/@angular/forms/src/directives/abstract_form_group_directive.ts +++ b/modules/@angular/forms/src/directives/abstract_form_group_directive.ts @@ -29,7 +29,10 @@ export class AbstractFormGroupDirective extends ControlContainer implements OnIn /** @internal */ _asyncValidators: any[]; - ngOnInit(): void { this.formDirective.addFormGroup(this); } + ngOnInit(): void { + this._checkParentType(); + this.formDirective.addFormGroup(this); + } ngOnDestroy(): void { this.formDirective.removeFormGroup(this); } @@ -51,4 +54,7 @@ export class AbstractFormGroupDirective extends ControlContainer implements OnIn get validator(): ValidatorFn { return composeValidators(this._validators); } get asyncValidator(): AsyncValidatorFn { return composeAsyncValidators(this._asyncValidators); } + + /** @internal */ + _checkParentType(): void {} } diff --git a/modules/@angular/forms/src/directives/reactive_directives/form_array_name.ts b/modules/@angular/forms/src/directives/reactive_directives/form_array_name.ts index 48b57edf4b..064667c163 100644 --- a/modules/@angular/forms/src/directives/reactive_directives/form_array_name.ts +++ b/modules/@angular/forms/src/directives/reactive_directives/form_array_name.ts @@ -8,6 +8,7 @@ import {Directive, Host, Inject, Input, OnDestroy, OnInit, Optional, Self, SkipSelf, forwardRef} from '@angular/core'; +import {BaseException} from '../../facade/exceptions'; import {FormArray} from '../../model'; import {NG_ASYNC_VALIDATORS, NG_VALIDATORS} from '../../validators'; import {ControlContainer} from '../control_container'; @@ -15,6 +16,7 @@ import {composeAsyncValidators, composeValidators, controlPath} from '../shared' import {AsyncValidatorFn, ValidatorFn} from '../validators'; import {FormGroupDirective} from './form_group_directive'; +import {FormGroupName} from './form_group_name'; export const formArrayNameProvider: any = /*@ts2dart_const*/ /* @ts2dart_Provider */ { @@ -71,7 +73,7 @@ export class FormArrayName extends ControlContainer implements OnInit, OnDestroy @Input('formArrayName') name: string; constructor( - @Host() @SkipSelf() parent: ControlContainer, + @Optional() @Host() @SkipSelf() parent: ControlContainer, @Optional() @Self() @Inject(NG_VALIDATORS) validators: any[], @Optional() @Self() @Inject(NG_ASYNC_VALIDATORS) asyncValidators: any[]) { super(); @@ -80,7 +82,10 @@ export class FormArrayName extends ControlContainer implements OnInit, OnDestroy this._asyncValidators = asyncValidators; } - ngOnInit(): void { this.formDirective.addFormArray(this); } + ngOnInit(): void { + this._checkParentType(); + this.formDirective.addFormArray(this); + } ngOnDestroy(): void { this.formDirective.removeFormArray(this); } @@ -93,4 +98,31 @@ export class FormArrayName extends ControlContainer implements OnInit, OnDestroy get validator(): ValidatorFn { return composeValidators(this._validators); } get asyncValidator(): AsyncValidatorFn { return composeAsyncValidators(this._asyncValidators); } + + private _checkParentType(): void { + if (!(this._parent instanceof FormGroupName) && !(this._parent instanceof FormGroupDirective)) { + this._throwParentException(); + } + } + + private _throwParentException(): void { + throw new BaseException(`formArrayName must be used with a parent formGroup directive. + You'll want to add a formGroup directive and pass it an existing FormGroup instance + (you can create one in your class). + + Example: +
+
+
+ +
+
+
+ + In your class: + this.cityArray = new FormArray([new FormControl('SF')]); + this.myGroup = new FormGroup({ + cities: this.cityArray + });`); + } } diff --git a/modules/@angular/forms/src/directives/reactive_directives/form_control_name.ts b/modules/@angular/forms/src/directives/reactive_directives/form_control_name.ts index 5579c575cf..a8b3ced01a 100644 --- a/modules/@angular/forms/src/directives/reactive_directives/form_control_name.ts +++ b/modules/@angular/forms/src/directives/reactive_directives/form_control_name.ts @@ -9,15 +9,19 @@ import {Directive, Host, Inject, Input, OnChanges, OnDestroy, Optional, Output, Self, SimpleChanges, SkipSelf, forwardRef} from '@angular/core'; import {EventEmitter, ObservableWrapper} from '../../facade/async'; +import {BaseException} from '../../facade/exceptions'; import {FormControl} from '../../model'; import {NG_ASYNC_VALIDATORS, NG_VALIDATORS} from '../../validators'; - import {ControlContainer} from '../control_container'; import {ControlValueAccessor, NG_VALUE_ACCESSOR} from '../control_value_accessor'; import {NgControl} from '../ng_control'; import {composeAsyncValidators, composeValidators, controlPath, isPropertyUpdated, selectValueAccessor} from '../shared'; import {AsyncValidatorFn, ValidatorFn} from '../validators'; +import {FormArrayName} from './form_array_name'; +import {FormGroupDirective} from './form_group_directive'; +import {FormGroupName} from './form_group_name'; + export const controlNameBinding: any = /*@ts2dart_const*/ /* @ts2dart_Provider */ { @@ -105,7 +109,7 @@ export class FormControlName extends NgControl implements OnChanges, OnDestroy { @Input('ngModel') model: any; @Output('ngModelChange') update = new EventEmitter(); - constructor(@Host() @SkipSelf() private _parent: ControlContainer, + constructor(@Optional() @Host() @SkipSelf() private _parent: ControlContainer, @Optional() @Self() @Inject(NG_VALIDATORS) private _validators: /* Array */ any[], @Optional() @Self() @Inject(NG_ASYNC_VALIDATORS) private _asyncValidators: @@ -118,6 +122,7 @@ export class FormControlName extends NgControl implements OnChanges, OnDestroy { ngOnChanges(changes: SimpleChanges) { if (!this._added) { + this._checkParentType(); this.formDirective.addControl(this); this._added = true; } @@ -145,4 +150,29 @@ export class FormControlName extends NgControl implements OnChanges, OnDestroy { } get control(): FormControl { return this.formDirective.getControl(this); } + + private _checkParentType(): void { + if (!(this._parent instanceof FormGroupName) && + !(this._parent instanceof FormGroupDirective) && + !(this._parent instanceof FormArrayName)) { + this._throwParentException(); + } + } + + private _throwParentException(): void { + throw new BaseException( + `formControlName must be used with a parent formGroup directive. + You'll want to add a formGroup directive and pass it an existing FormGroup instance + (you can create one in your class). + + Example: +
+ +
+ + In your class: + this.myGroup = new FormGroup({ + firstName: new FormControl() + });`); + } } diff --git a/modules/@angular/forms/src/directives/reactive_directives/form_group_name.ts b/modules/@angular/forms/src/directives/reactive_directives/form_group_name.ts index 779bc873d2..bfe00f0975 100644 --- a/modules/@angular/forms/src/directives/reactive_directives/form_group_name.ts +++ b/modules/@angular/forms/src/directives/reactive_directives/form_group_name.ts @@ -7,10 +7,14 @@ */ import {Directive, Host, Inject, Input, OnDestroy, OnInit, Optional, Self, SkipSelf, forwardRef} from '@angular/core'; + +import {BaseException} from '../../facade/exceptions'; import {NG_ASYNC_VALIDATORS, NG_VALIDATORS} from '../../validators'; import {AbstractFormGroupDirective} from '../abstract_form_group_directive'; import {ControlContainer} from '../control_container'; +import {FormGroupDirective} from './form_group_directive'; + export const formGroupNameProvider: any = /*@ts2dart_const*/ /* @ts2dart_Provider */ { provide: ControlContainer, @@ -70,7 +74,7 @@ export class FormGroupName extends AbstractFormGroupDirective implements OnInit, @Input('formGroupName') name: string; constructor( - @Host() @SkipSelf() parent: ControlContainer, + @Optional() @Host() @SkipSelf() parent: ControlContainer, @Optional() @Self() @Inject(NG_VALIDATORS) validators: any[], @Optional() @Self() @Inject(NG_ASYNC_VALIDATORS) asyncValidators: any[]) { super(); @@ -78,4 +82,29 @@ export class FormGroupName extends AbstractFormGroupDirective implements OnInit, this._validators = validators; this._asyncValidators = asyncValidators; } + + /** @internal */ + _checkParentType(): void { + if (!(this._parent instanceof FormGroupName) && !(this._parent instanceof FormGroupDirective)) { + this._throwParentException(); + } + } + + private _throwParentException() { + throw new BaseException(`formGroupName must be used with a parent formGroup directive. + You'll want to add a formGroup directive and pass it an existing FormGroup instance + (you can create one in your class). + + Example: +
+
+ +
+
+ + In your class: + this.myGroup = new FormGroup({ + person: new FormGroup({ firstName: new FormControl() }) + });`); + } } diff --git a/modules/@angular/forms/test/reactive_integration_spec.ts b/modules/@angular/forms/test/reactive_integration_spec.ts index aca8273d8b..492eb481e4 100644 --- a/modules/@angular/forms/test/reactive_integration_spec.ts +++ b/modules/@angular/forms/test/reactive_integration_spec.ts @@ -43,21 +43,6 @@ export function main() { }); })); - it('should throw if a form isn\'t passed into formGroup', - inject( - [TestComponentBuilder, AsyncTestCompleter], - (tcb: TestComponentBuilder, async: AsyncTestCompleter) => { - const t = `
- -
`; - - tcb.overrideTemplate(MyComp8, t).createAsync(MyComp8).then((fixture) => { - expect(() => fixture.detectChanges()) - .toThrowError(new RegExp(`formGroup expects a FormGroup instance`)); - async.done(); - }); - })); - it('should update the form group values on DOM change', inject( [TestComponentBuilder, AsyncTestCompleter], @@ -623,23 +608,6 @@ export function main() { }); })); - it('should throw if radio button name does not match formControlName attr', - inject( - [TestComponentBuilder, AsyncTestCompleter], - (tcb: TestComponentBuilder, async: AsyncTestCompleter) => { - const t = `
- -
`; - - tcb.overrideTemplate(MyComp8, t).createAsync(MyComp8).then((fixture) => { - fixture.debugElement.componentInstance.form = - new FormGroup({'food': new FormControl('fish')}); - expect(() => fixture.detectChanges()) - .toThrowError(new RegExp('If you define both a name and a formControlName')); - async.done(); - }); - })); - it('should support removing controls from ', inject( [TestComponentBuilder, AsyncTestCompleter], @@ -1162,6 +1130,139 @@ export function main() { // selection start has not changed because we did not reset the value expect(input.selectionStart).toEqual(1); }))); + + describe('errors', () => { + + it('should throw if a form isn\'t passed into formGroup', + inject( + [TestComponentBuilder, AsyncTestCompleter], + (tcb: TestComponentBuilder, async: AsyncTestCompleter) => { + const t = `
+ +
`; + + tcb.overrideTemplate(MyComp8, t).createAsync(MyComp8).then((fixture) => { + expect(() => fixture.detectChanges()) + .toThrowError(new RegExp(`formGroup expects a FormGroup instance`)); + async.done(); + }); + })); + + it('should throw if formControlName is used without a control container', + inject( + [TestComponentBuilder, AsyncTestCompleter], + (tcb: TestComponentBuilder, async: AsyncTestCompleter) => { + const t = ``; + + tcb.overrideTemplate(MyComp8, t).createAsync(MyComp8).then((fixture) => { + expect(() => fixture.detectChanges()) + .toThrowError(new RegExp( + `formControlName must be used with a parent formGroup directive`)); + async.done(); + }); + })); + + it('should throw if formGroupName is used without a control container', + inject( + [TestComponentBuilder, AsyncTestCompleter], + (tcb: TestComponentBuilder, async: AsyncTestCompleter) => { + const t = `
+ +
`; + + tcb.overrideTemplate(MyComp8, t).createAsync(MyComp8).then((fixture) => { + expect(() => fixture.detectChanges()) + .toThrowError(new RegExp( + `formGroupName must be used with a parent formGroup directive`)); + async.done(); + }); + })); + + it('should throw if formArrayName is used without a control container', + inject( + [TestComponentBuilder, AsyncTestCompleter], + (tcb: TestComponentBuilder, async: AsyncTestCompleter) => { + const t = `
+ +
`; + + tcb.overrideTemplate(MyComp8, t).createAsync(MyComp8).then((fixture) => { + expect(() => fixture.detectChanges()) + .toThrowError(new RegExp( + `formArrayName must be used with a parent formGroup directive`)); + async.done(); + }); + })); + + it('should throw if formControlName is used with the wrong control container', + inject( + [TestComponentBuilder, AsyncTestCompleter], + (tcb: TestComponentBuilder, async: AsyncTestCompleter) => { + const t = `
+ +
`; + + tcb.overrideTemplate(MyComp8, t).createAsync(MyComp8).then((fixture) => { + expect(() => fixture.detectChanges()) + .toThrowError(new RegExp( + `formControlName must be used with a parent formGroup directive.`)); + async.done(); + }); + })); + + it('should throw if formGroupName is used with the wrong control container', + inject( + [TestComponentBuilder, AsyncTestCompleter], + (tcb: TestComponentBuilder, async: AsyncTestCompleter) => { + const t = `
+
+ +
+
`; + + tcb.overrideTemplate(MyComp8, t).createAsync(MyComp8).then((fixture) => { + expect(() => fixture.detectChanges()) + .toThrowError(new RegExp( + `formGroupName must be used with a parent formGroup directive.`)); + async.done(); + }); + })); + + it('should throw if formArrayName is used with the wrong control container', + inject( + [TestComponentBuilder, AsyncTestCompleter], + (tcb: TestComponentBuilder, async: AsyncTestCompleter) => { + const t = `
+
+ +
+
`; + + tcb.overrideTemplate(MyComp8, t).createAsync(MyComp8).then((fixture) => { + expect(() => fixture.detectChanges()) + .toThrowError(new RegExp( + `formArrayName must be used with a parent formGroup directive.`)); + async.done(); + }); + })); + + it('should throw if radio button name does not match formControlName attr', + inject( + [TestComponentBuilder, AsyncTestCompleter], + (tcb: TestComponentBuilder, async: AsyncTestCompleter) => { + const t = `
+ +
`; + + tcb.overrideTemplate(MyComp8, t).createAsync(MyComp8).then((fixture) => { + fixture.debugElement.componentInstance.form = + new FormGroup({'food': new FormControl('fish')}); + expect(() => fixture.detectChanges()) + .toThrowError(new RegExp('If you define both a name and a formControlName')); + async.done(); + }); + })); + }); }); }