feat(forms): Validator.pattern accepts a RegExp #12323

Merged
merged 1 commit into from Oct 19, 2016

Conversation

Projects
None yet
7 participants
@DzmitryShylovich
Contributor

DzmitryShylovich commented Oct 15, 2016

Fixes #10150

@googlebot googlebot added the cla: yes label Oct 15, 2016

modules/@angular/forms/src/validators.ts
@@ -7,6 +7,7 @@
*/
import {OpaqueToken} from '@angular/core';
+import {isString} from '@angular/facade/src/lang';

This comment has been minimized.

@pkozlowski-opensource

pkozlowski-opensource Oct 15, 2016

Member

Please don't use facades, we are in the process of removing them

@pkozlowski-opensource

pkozlowski-opensource Oct 15, 2016

Member

Please don't use facades, we are in the process of removing them

modules/@angular/forms/src/validators.ts
return (control: AbstractControl): {[key: string]: any} => {
if (isEmptyInputValue(control.value)) {
return null; // don't validate empty values to allow optional controls
}
- const regex = new RegExp(`^${pattern}$`);
+ const regex: RegExp = isString(pattern) ? new RegExp(`^${pattern}$`) : <RegExp>pattern;

This comment has been minimized.

@pkozlowski-opensource

pkozlowski-opensource Oct 15, 2016

Member

Is the type necessary in const regex: RegExp?

@pkozlowski-opensource

pkozlowski-opensource Oct 15, 2016

Member

Is the type necessary in const regex: RegExp?

@vicb vicb added the comp: forms label Oct 15, 2016

modules/@angular/forms/src/validators.ts
return (control: AbstractControl): {[key: string]: any} => {
if (isEmptyInputValue(control.value)) {
return null; // don't validate empty values to allow optional controls
}
- const regex = new RegExp(`^${pattern}$`);
+ const regex = typeof pattern === 'string' ? new RegExp(`^${pattern}$`) : <RegExp>pattern;

This comment has been minimized.

@vicb

vicb Oct 16, 2016

Member

<RegExp> should not be needed

@vicb

vicb Oct 16, 2016

Member

<RegExp> should not be needed

This comment has been minimized.

@DzmitryShylovich

DzmitryShylovich Oct 16, 2016

Contributor

tsc complains (at least in ide)

@DzmitryShylovich

DzmitryShylovich Oct 16, 2016

Contributor

tsc complains (at least in ide)

modules/@angular/forms/src/validators.ts
const value: string = control.value;
- return regex.test(value) ?
+ return regex && regex.test(value) ?

This comment has been minimized.

@vicb

vicb Oct 16, 2016

Member

not sure of the !regex case ?
Should it rather returns an empty ValidatorFn ? (_) => null

@vicb

vicb Oct 16, 2016

Member

not sure of the !regex case ?
Should it rather returns an empty ValidatorFn ? (_) => null

This comment has been minimized.

@vicb

vicb Oct 16, 2016

Member

Add a test

@vicb

vicb Oct 16, 2016

Member

Add a test

it('should error on failure to match string', () => {
expect(Validators.pattern('[a-zA-Z ]*')(new FormControl('aaa0'))).toEqual({
- 'pattern': {'requiredPattern': '^[a-zA-Z ]*$', 'actualValue': 'aaa0'}
+ 'pattern': {'requiredPattern': '/^[a-zA-Z ]*$/', 'actualValue': 'aaa0'}

This comment has been minimized.

@vicb

vicb Oct 16, 2016

Member

Could you please remove this breaking change ? (we would have to delay the PR until 3.0). Thanks

@vicb

vicb Oct 16, 2016

Member

Could you please remove this breaking change ? (we would have to delay the PR until 3.0). Thanks

modules/@angular/forms/src/validators.ts
const value: string = control.value;
return regex.test(value) ?
null :
- {'pattern': {'requiredPattern': `^${pattern}$`, 'actualValue': value}};
+ {'pattern': {'requiredPattern': `${regexStr}`, 'actualValue': value}};

This comment has been minimized.

@arkon

arkon Oct 17, 2016

Why not just directly put regexStr?

@arkon

arkon Oct 17, 2016

Why not just directly put regexStr?

@alxhub alxhub merged commit bf60418 into angular:master Oct 19, 2016

2 of 3 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
ci/circleci Your tests passed on CircleCI!
Details
cla/google All necessary CLAs are signed

btrigueiro added a commit to btrigueiro/angular that referenced this pull request Oct 21, 2016

@DzmitryShylovich DzmitryShylovich deleted the DzmitryShylovich:feature/10150 branch Oct 28, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment