fix(forms): accept number length in length validators by cexbrayat · Pull Request #32057 · angular/angular

cexbrayat · 2019-08-08T17:16:48Z

PR Checklist

Please check if your PR fulfills the following requirements:

The commit message follows our guidelines: https://github.com/angular/angular/blob/master/CONTRIBUTING.md#commit
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

Bugfix
Feature
Code style update (formatting, local variables)
Refactoring (no functional changes, no api changes)
Build related changes
CI related changes
Documentation content changes
angular.io application / infrastructure changes
Other... Please describe:

What is the current behavior?

Both MinLengthValidator and MaxLengthValidator accepted only string inputs for the length required, which throws with Ivy and fullTemplateTypeCheck enabled:

<!-- min = 2 in the component -->
<input [minlength]="min">

with:

Type 'number' is not assignable to type 'string | undefined'

What is the new behavior?

This relaxes the accepted type to string | number to avoid breakage when developers switch to Ivy and fTTC.

Does this PR introduce a breaking change?

Yes
No

If someone extends one of these validators in their own code, then yes, they'll have to pdate the type of the minlength/maxlength field.

Other information

googlebot · 2019-08-08T17:16:51Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

googlebot · 2019-08-08T17:20:00Z

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

kara · 2019-08-29T19:59:13Z

VE presubmit

global TAP

ngbot · 2019-11-08T17:13:36Z

I see that you just added the PR action: merge label, but the following checks are still failing:
status "ci-codefresh-bazel" is failing

If you want your PR to be merged, it has to pass all the CI checks.

If you can't get the PR to a green state due to flakes or broken master, please try rebasing to master and/or restarting the CI job. If that fails and you believe that the issue is not due to your change, please contact the caretaker and ask for help.

mhevery · 2019-11-08T17:30:23Z

presubmit

atscott · 2020-01-13T18:37:24Z

presubmit

Both `MinLengthValidator` and `MaxLengthValidator` accepted only string inputs for the length required, which throws with Ivy and `fullTemplateTypeCheck` enabled:  <input [minlength]="min"> with: Type 'number' is not assignable to type 'string | undefined' This relaxes the accepted type to `string | number` to avoid breakage when developers switch to Ivy and fTTC.

AndrewKushnir

The change LGTM, but I think we need to check whether it should go to v9 or v10. Thank you.

AndrewKushnir · 2020-01-13T23:02:41Z

FYI, I've put "blocked" label for now to make sure that we merge it once the decision is made on when this PR should be merged (v9 or v10).

IgorMinar

lgtm for api change. this is minor enough that it can go to v9.0.

thanks @cexbrayat!

IgorMinar · 2020-01-14T22:54:51Z

For the record, we don't consider this to be a breaking change for public api, because we discourage people from extend our classes unless we documented that the class is meant to be subclassed:

extending any of our classes unless the support for this is specifically documented in the API docs
from: https://github.com/angular/angular/blob/master/docs/PUBLIC_API.md

Both `MinLengthValidator` and `MaxLengthValidator` accepted only string inputs for the length required, which throws with Ivy and `fullTemplateTypeCheck` enabled:  <input [minlength]="min"> with: Type 'number' is not assignable to type 'string | undefined' This relaxes the accepted type to `string | number` to avoid breakage when developers switch to Ivy and fTTC. PR Close #32057

angular-automatic-lock-bot · 2020-02-14T16:09:07Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

cexbrayat requested review from a team August 8, 2019 17:16

googlebot added the cla: no label Aug 8, 2019

cexbrayat force-pushed the feat/ivy-maxlength branch from 8a143fa to 177ba2c Compare August 8, 2019 17:19

googlebot added cla: yes and removed cla: no labels Aug 8, 2019

cexbrayat force-pushed the feat/ivy-maxlength branch from 177ba2c to 649ffdc Compare August 8, 2019 18:14

alxhub added the area: forms label Aug 8, 2019

ngbot bot added this to the needsTriage milestone Aug 8, 2019

petebacondarwin approved these changes Aug 9, 2019

View reviewed changes

kara added breaking changes action: presubmit The PR is in need of a google3 presubmit action: review The PR is still awaiting reviews from at least one requested reviewer target: major This PR is targeted for the next major release labels Aug 29, 2019

mhevery added action: merge The PR is ready for merge by the caretaker and removed action: presubmit The PR is in need of a google3 presubmit action: review The PR is still awaiting reviews from at least one requested reviewer labels Nov 8, 2019

mhevery force-pushed the feat/ivy-maxlength branch from 649ffdc to 0b85cd2 Compare November 8, 2019 17:14

kara added the action: presubmit The PR is in need of a google3 presubmit label Nov 9, 2019

alxhub added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews and removed action: presubmit The PR is in need of a google3 presubmit labels Nov 12, 2019

cexbrayat force-pushed the feat/ivy-maxlength branch from 0b85cd2 to 12f70c7 Compare November 12, 2019 20:20

cexbrayat changed the title ~~feat(forms): accept number length in length validators~~ fix(forms): accept number length in length validators Nov 12, 2019

alxhub removed the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Nov 12, 2019

alxhub added target: patch This PR is targeted for the next patch release and removed target: major This PR is targeted for the next major release labels Nov 12, 2019

cexbrayat force-pushed the feat/ivy-maxlength branch from 12f70c7 to f96e98d Compare November 12, 2019 21:10

IgorMinar modified the milestones: needsTriage, v10-candidates Nov 27, 2019

alxhub added the PR action: rerun CI at HEAD label Jan 6, 2020

ngbot bot removed the PR action: rerun CI at HEAD label Jan 6, 2020

atscott added the PR action: rerun CI at HEAD label Jan 13, 2020

ngbot bot removed the PR action: rerun CI at HEAD label Jan 13, 2020

atscott force-pushed the feat/ivy-maxlength branch from f96e98d to 8dd6d47 Compare January 13, 2020 22:14

AndrewKushnir approved these changes Jan 13, 2020

View reviewed changes

AndrewKushnir added the state: blocked label Jan 13, 2020

IgorMinar approved these changes Jan 14, 2020

View reviewed changes

IgorMinar removed the state: blocked label Jan 14, 2020

IgorMinar removed the breaking changes label Jan 14, 2020

atscott closed this in 9ceee07 Jan 14, 2020

angular-automatic-lock-bot bot locked and limited conversation to collaborators Feb 14, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(forms): accept number length in length validators#32057

fix(forms): accept number length in length validators#32057
cexbrayat wants to merge 1 commit intoangular:masterfrom
cexbrayat:feat/ivy-maxlength

cexbrayat commented Aug 8, 2019

Uh oh!

googlebot commented Aug 8, 2019

Uh oh!

googlebot commented Aug 8, 2019

Uh oh!

kara commented Aug 29, 2019 •

edited

Loading

Uh oh!

ngbot bot commented Nov 8, 2019

Uh oh!

mhevery commented Nov 8, 2019

Uh oh!

atscott commented Jan 13, 2020

Uh oh!

AndrewKushnir left a comment

Uh oh!

AndrewKushnir commented Jan 13, 2020

Uh oh!

IgorMinar left a comment

Uh oh!

IgorMinar commented Jan 14, 2020

Uh oh!

angular-automatic-lock-bot bot commented Feb 14, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

Conversation

cexbrayat commented Aug 8, 2019

PR Checklist

PR Type

What is the current behavior?

What is the new behavior?

Does this PR introduce a breaking change?

Other information

Uh oh!

googlebot commented Aug 8, 2019

Uh oh!

googlebot commented Aug 8, 2019

Uh oh!

kara commented Aug 29, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ngbot bot commented Nov 8, 2019

Uh oh!

mhevery commented Nov 8, 2019

Uh oh!

atscott commented Jan 13, 2020

Uh oh!

AndrewKushnir left a comment

Choose a reason for hiding this comment

Uh oh!

AndrewKushnir commented Jan 13, 2020

Uh oh!

IgorMinar left a comment

Choose a reason for hiding this comment

Uh oh!

IgorMinar commented Jan 14, 2020

Uh oh!

angular-automatic-lock-bot bot commented Feb 14, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

kara commented Aug 29, 2019 •

edited

Loading