Skip to content

refactor(forms): match getError and hasError to get method signature #20211

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Toxicable wants to merge 1 commit into from
Closed

refactor(forms): match getError and hasError to get method signature #20211

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/forms/src/directives/abstract_control_directive.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ export abstract class AbstractControlDirective {
* If no path is given, it checks for the error on the present control.
* If the control is not present, false is returned.
*/
hasError(errorCode: string, path?: string[]): boolean {
hasError(errorCode: string, path?: Array<string|number>|string): boolean {
return this.control ? this.control.hasError(errorCode, path) : false;
}

Expand All @@ -162,7 +162,7 @@ export abstract class AbstractControlDirective {
* Reports error data for the control with the given path.
* If the control is not present, null is returned.
*/
getError(errorCode: string, path?: string[]): any {
getError(errorCode: string, path?: Array<string|number>|string): any {
return this.control ? this.control.getError(errorCode, path) : null;
}
}
6 changes: 4 additions & 2 deletions packages/forms/src/model.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -654,7 +654,7 @@ export abstract class AbstractControl {
* @returns The error data if the control with the given path has the given error, otherwise null
* or undefined.
*/
getError(errorCode: string, path?: string[]): any {
getError(errorCode: string, path?: Array<string|number>|string): any {
const control = path ? this.get(path) : this;
return control && control.errors ? control.errors[errorCode] : null;
}
Expand All @@ -667,7 +667,9 @@ export abstract class AbstractControl {
* control.
* @returns True when the control with the given path has the error, otherwise false.
*/
hasError(errorCode: string, path?: string[]): boolean { return !!this.getError(errorCode, path); }
hasError(errorCode: string, path?: Array<string|number>|string): boolean {

@IgorMinar IgorMinar Feb 8, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why number? Seems odd. And there are no tests as far as I can tell for using numerical arrays.

@Toxicable Toxicable Feb 8, 2018 •
edited
Loading

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seams valid if you're traversing through a form array - form arrya being index based.
Ill add some tests.

@kara kara Nov 30, 2018 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you update the type signature on AbstractControlDirective for getError and hasError as well? Looks like it's inconsistent between AbstractControl and AbstractControlDirective.

@Toxicable Toxicable Nov 30, 2018

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh yes, will get on that

return !!this.getError(errorCode, path);
}

/**
* Retrieves the top-level ancestor of this control.
Expand Down
64 changes: 64 additions & 0 deletions packages/forms/test/form_group_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -684,6 +684,70 @@ import {of } from 'rxjs';
expect(g.getError('required', ['one'])).toEqual(null);
expect(g.getError('required', ['invalid'])).toEqual(null);
});

it('should be able to traverse group with single string', () => {
const c = new FormControl('', Validators.required);
const g = new FormGroup({'one': c});
expect(c.getError('required')).toEqual(true);
expect(g.getError('required', 'one')).toEqual(true);
});

it('should be able to traverse group with string delimited by dots', () => {
const c = new FormControl('', Validators.required);
const g2 = new FormGroup({'two': c});
const g1 = new FormGroup({'one': g2});
expect(c.getError('required')).toEqual(true);
expect(g1.getError('required', 'one.two')).toEqual(true);
});

it('should traverse group with form array using string and numbers', () => {
const c = new FormControl('', Validators.required);
const g2 = new FormGroup({'two': c});
const a = new FormArray([g2]);
const g1 = new FormGroup({'one': a});
expect(c.getError('required')).toEqual(true);
expect(g1.getError('required', ['one', 0, 'two'])).toEqual(true);
});
});

describe('hasError', () => {
it('should return true when it is present', () => {
const c = new FormControl('', Validators.required);
const g = new FormGroup({'one': c});
expect(c.hasError('required')).toEqual(true);
expect(g.hasError('required', ['one'])).toEqual(true);
});

it('should return false otherwise', () => {
const c = new FormControl('not empty', Validators.required);
const g = new FormGroup({'one': c});
expect(c.hasError('invalid')).toEqual(false);
expect(g.hasError('required', ['one'])).toEqual(false);
expect(g.hasError('required', ['invalid'])).toEqual(false);
});

it('should be able to traverse group with single string', () => {
const c = new FormControl('', Validators.required);
const g = new FormGroup({'one': c});
expect(c.hasError('required')).toEqual(true);
expect(g.hasError('required', 'one')).toEqual(true);
});

it('should be able to traverse group with string delimited by dots', () => {
const c = new FormControl('', Validators.required);
const g2 = new FormGroup({'two': c});
const g1 = new FormGroup({'one': g2});
expect(c.hasError('required')).toEqual(true);
expect(g1.hasError('required', 'one.two')).toEqual(true);
});
it('should traverse group with form array using string and numbers', () => {
const c = new FormControl('', Validators.required);
const g2 = new FormGroup({'two': c});
const a = new FormArray([g2]);
const g1 = new FormGroup({'one': a});
expect(c.getError('required')).toEqual(true);
expect(g1.getError('required', ['one', 0, 'two'])).toEqual(true);
});
});

describe('validator', () => {
Expand Down
8 changes: 4 additions & 4 deletions tools/public_api_guard/forms/forms.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ export declare abstract class AbstractControl {
emitEvent?: boolean;
}): void;
get(path: Array<string | number> | string): AbstractControl | null;
getError(errorCode: string, path?: string[]): any;
hasError(errorCode: string, path?: string[]): boolean;
getError(errorCode: string, path?: Array<string | number> | string): any;
hasError(errorCode: string, path?: Array<string | number> | string): boolean;
markAsDirty(opts?: {
onlySelf?: boolean;
}): void;
Expand Down Expand Up @@ -80,8 +80,8 @@ export declare abstract class AbstractControlDirective {
readonly valid: boolean | null;
readonly value: any;
readonly valueChanges: Observable<any> | null;
getError(errorCode: string, path?: string[]): any;
hasError(errorCode: string, path?: string[]): boolean;
getError(errorCode: string, path?: Array<string | number> | string): any;
hasError(errorCode: string, path?: Array<string | number> | string): boolean;
reset(value?: any): void;
}

Expand Down