From 3e685f98c666fcb9f0556ccb65d4b40716ac2985 Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Mon, 19 Jun 2017 16:26:43 -0700 Subject: [PATCH] fix(forms): roll back breaking change with min/max directives With 4.2, we introduced the min and max validator directives. This was actually a breaking change because their selectors could include custom value accessors using the min/max properties for their own purposes. For now, we are rolling back the change by removing the exports. At the least, we should wait to add them until a major version. In the meantime, we will have further discussion about what the best solution is going forward for all validator directives. Closes #17491. ---- PR #17551 tried to roll this back, but did not remove the dead code. This failed internal tests that were checking that all declared directives were used. This PR rolls back the original PR and commit the same as #17551 while also removing the dead code. --- packages/forms/src/directives.ts | 4 +- packages/forms/src/directives/validators.ts | 77 ------------- packages/forms/src/forms.ts | 2 +- .../forms/test/template_integration_spec.ts | 107 ------------------ tools/public_api_guard/forms/forms.d.ts | 16 --- 5 files changed, 2 insertions(+), 204 deletions(-) diff --git a/packages/forms/src/directives.ts b/packages/forms/src/directives.ts index 101a6103c9..41a298e90e 100644 --- a/packages/forms/src/directives.ts +++ b/packages/forms/src/directives.ts @@ -24,7 +24,7 @@ import {FormGroupDirective} from './directives/reactive_directives/form_group_di import {FormArrayName, FormGroupName} from './directives/reactive_directives/form_group_name'; import {NgSelectOption, SelectControlValueAccessor} from './directives/select_control_value_accessor'; import {NgSelectMultipleOption, SelectMultipleControlValueAccessor} from './directives/select_multiple_control_value_accessor'; -import {CheckboxRequiredValidator, EmailValidator, MaxLengthValidator, MaxValidator, MinLengthValidator, MinValidator, PatternValidator, RequiredValidator} from './directives/validators'; +import {CheckboxRequiredValidator, EmailValidator, MaxLengthValidator, MinLengthValidator, PatternValidator, RequiredValidator} from './directives/validators'; export {CheckboxControlValueAccessor} from './directives/checkbox_value_accessor'; export {ControlValueAccessor} from './directives/control_value_accessor'; @@ -58,9 +58,7 @@ export const SHARED_FORM_DIRECTIVES: Type[] = [ NgControlStatus, NgControlStatusGroup, RequiredValidator, - MinValidator, MinLengthValidator, - MaxValidator, MaxLengthValidator, PatternValidator, CheckboxRequiredValidator, diff --git a/packages/forms/src/directives/validators.ts b/packages/forms/src/directives/validators.ts index 7dddf3a3c3..7ec13a6208 100644 --- a/packages/forms/src/directives/validators.ts +++ b/packages/forms/src/directives/validators.ts @@ -95,83 +95,6 @@ export class RequiredValidator implements Validator { registerOnValidatorChange(fn: () => void): void { this._onChange = fn; } } -export const MIN_VALIDATOR: Provider = { - provide: NG_VALIDATORS, - useExisting: forwardRef(() => MinValidator), - multi: true -}; - -/** - * A directive which installs the {@link MinValidator} for any `formControlName`, - * `formControl`, or control with `ngModel` that also has a `min` attribute. - * - * @experimental - */ -@Directive({ - selector: '[min][formControlName],[min][formControl],[min][ngModel]', - providers: [MIN_VALIDATOR], - host: {'[attr.min]': 'min ? min : null'} -}) -export class MinValidator implements Validator, - OnChanges { - private _validator: ValidatorFn; - private _onChange: () => void; - - @Input() min: string; - - ngOnChanges(changes: SimpleChanges): void { - if ('min' in changes) { - this._createValidator(); - if (this._onChange) this._onChange(); - } - } - - validate(c: AbstractControl): ValidationErrors|null { return this._validator(c); } - - registerOnValidatorChange(fn: () => void): void { this._onChange = fn; } - - private _createValidator(): void { this._validator = Validators.min(parseInt(this.min, 10)); } -} - - -export const MAX_VALIDATOR: Provider = { - provide: NG_VALIDATORS, - useExisting: forwardRef(() => MaxValidator), - multi: true -}; - -/** - * A directive which installs the {@link MaxValidator} for any `formControlName`, - * `formControl`, or control with `ngModel` that also has a `min` attribute. - * - * @experimental - */ -@Directive({ - selector: '[max][formControlName],[max][formControl],[max][ngModel]', - providers: [MAX_VALIDATOR], - host: {'[attr.max]': 'max ? max : null'} -}) -export class MaxValidator implements Validator, - OnChanges { - private _validator: ValidatorFn; - private _onChange: () => void; - - @Input() max: string; - - ngOnChanges(changes: SimpleChanges): void { - if ('max' in changes) { - this._createValidator(); - if (this._onChange) this._onChange(); - } - } - - validate(c: AbstractControl): ValidationErrors|null { return this._validator(c); } - - registerOnValidatorChange(fn: () => void): void { this._onChange = fn; } - - private _createValidator(): void { this._validator = Validators.max(parseInt(this.max, 10)); } -} - /** * A Directive that adds the `required` validator to checkbox controls marked with the diff --git a/packages/forms/src/forms.ts b/packages/forms/src/forms.ts index b1f882d1d4..81caaa2dca 100644 --- a/packages/forms/src/forms.ts +++ b/packages/forms/src/forms.ts @@ -38,7 +38,7 @@ export {FormArrayName} from './directives/reactive_directives/form_group_name'; export {FormGroupName} from './directives/reactive_directives/form_group_name'; export {NgSelectOption, SelectControlValueAccessor} from './directives/select_control_value_accessor'; export {SelectMultipleControlValueAccessor} from './directives/select_multiple_control_value_accessor'; -export {AsyncValidator, AsyncValidatorFn, CheckboxRequiredValidator, EmailValidator, MaxLengthValidator, MaxValidator, MinLengthValidator, MinValidator, PatternValidator, RequiredValidator, ValidationErrors, Validator, ValidatorFn} from './directives/validators'; +export {AsyncValidator, AsyncValidatorFn, CheckboxRequiredValidator, EmailValidator, MaxLengthValidator, MinLengthValidator, PatternValidator, RequiredValidator, ValidationErrors, Validator, ValidatorFn} from './directives/validators'; export {FormBuilder} from './form_builder'; export {AbstractControl, FormArray, FormControl, FormGroup} from './model'; export {NG_ASYNC_VALIDATORS, NG_VALIDATORS, Validators} from './validators'; diff --git a/packages/forms/test/template_integration_spec.ts b/packages/forms/test/template_integration_spec.ts index 9368dd859a..87fd77bdf8 100644 --- a/packages/forms/test/template_integration_spec.ts +++ b/packages/forms/test/template_integration_spec.ts @@ -879,97 +879,6 @@ export function main() { describe('validation directives', () => { - it('should should validate max', fakeAsync(() => { - const fixture = initTest(NgModelMaxValidator); - fixture.componentInstance.max = 10; - fixture.detectChanges(); - tick(); - - const max = fixture.debugElement.query(By.css('input')); - const form = fixture.debugElement.children[0].injector.get(NgForm); - - max.nativeElement.value = ''; - dispatchEvent(max.nativeElement, 'input'); - fixture.detectChanges(); - expect(form.valid).toEqual(true); - - max.nativeElement.value = 11; - dispatchEvent(max.nativeElement, 'input'); - fixture.detectChanges(); - expect(form.valid).toEqual(false); - - max.nativeElement.value = 9; - dispatchEvent(max.nativeElement, 'input'); - fixture.detectChanges(); - expect(form.valid).toEqual(true); - })); - - it('should validate max for strings', fakeAsync(() => { - const fixture = initTest(NgModelMaxValidator); - fixture.componentInstance.max = 10; - fixture.detectChanges(); - tick(); - - const max = fixture.debugElement.query(By.css('input')); - const form = fixture.debugElement.children[0].injector.get(NgForm); - - max.nativeElement.value = '11'; - dispatchEvent(max.nativeElement, 'input'); - fixture.detectChanges(); - expect(form.valid).toEqual(false); - - max.nativeElement.value = '9'; - dispatchEvent(max.nativeElement, 'input'); - fixture.detectChanges(); - expect(form.valid).toEqual(true); - })); - - it('should should validate min', fakeAsync(() => { - const fixture = initTest(NgModelMinValidator); - fixture.componentInstance.min = 10; - fixture.detectChanges(); - tick(); - - const min = fixture.debugElement.query(By.css('input')); - const form = fixture.debugElement.children[0].injector.get(NgForm); - - min.nativeElement.value = ''; - dispatchEvent(min.nativeElement, 'input'); - fixture.detectChanges(); - expect(form.valid).toEqual(true); - - min.nativeElement.value = 11; - dispatchEvent(min.nativeElement, 'input'); - fixture.detectChanges(); - expect(form.valid).toEqual(true); - - min.nativeElement.value = 9; - dispatchEvent(min.nativeElement, 'input'); - fixture.detectChanges(); - expect(form.valid).toEqual(false); - })); - - it('should should validate min for strings', fakeAsync(() => { - const fixture = initTest(NgModelMinValidator); - fixture.componentInstance.min = 10; - fixture.detectChanges(); - tick(); - - const min = fixture.debugElement.query(By.css('input')); - const form = fixture.debugElement.children[0].injector.get(NgForm); - - min.nativeElement.value = '11'; - dispatchEvent(min.nativeElement, 'input'); - fixture.detectChanges(); - expect(form.valid).toEqual(true); - - min.nativeElement.value = '9'; - dispatchEvent(min.nativeElement, 'input'); - fixture.detectChanges(); - expect(form.valid).toEqual(false); - })); - - it('required validator should validate checkbox', fakeAsync(() => { const fixture = initTest(NgModelCheckboxRequiredValidator); fixture.detectChanges(); @@ -1630,22 +1539,6 @@ class NgModelMultipleValidators { pattern: string|RegExp; } -@Component({ - selector: 'ng-model-max', - template: `
` -}) -class NgModelMaxValidator { - max: number; -} - -@Component({ - selector: 'ng-model-min', - template: `
` -}) -class NgModelMinValidator { - min: number; -} - @Component({ selector: 'ng-model-checkbox-validator', template: diff --git a/tools/public_api_guard/forms/forms.d.ts b/tools/public_api_guard/forms/forms.d.ts index 398d80c064..5015340bef 100644 --- a/tools/public_api_guard/forms/forms.d.ts +++ b/tools/public_api_guard/forms/forms.d.ts @@ -352,14 +352,6 @@ export declare class MaxLengthValidator implements Validator, OnChanges { validate(c: AbstractControl): ValidationErrors | null; } -/** @experimental */ -export declare class MaxValidator implements Validator, OnChanges { - max: string; - ngOnChanges(changes: SimpleChanges): void; - registerOnValidatorChange(fn: () => void): void; - validate(c: AbstractControl): ValidationErrors | null; -} - /** @stable */ export declare class MinLengthValidator implements Validator, OnChanges { minlength: string; @@ -368,14 +360,6 @@ export declare class MinLengthValidator implements Validator, OnChanges { validate(c: AbstractControl): ValidationErrors | null; } -/** @experimental */ -export declare class MinValidator implements Validator, OnChanges { - min: string; - ngOnChanges(changes: SimpleChanges): void; - registerOnValidatorChange(fn: () => void): void; - validate(c: AbstractControl): ValidationErrors | null; -} - /** @stable */ export declare const NG_ASYNC_VALIDATORS: InjectionToken<(Function | Validator)[]>;