Hi @laogao

Thanks for the issue. I understand that you would like to validate that an array, map, or any other type of Iterable must have at least one element.

You can achieve that with the current validator ValidationMessage.minLength. You can just define ValidationMessage.minLength(1) to specify there should be at least 1 item in the Iterable. This validator also takes into account FormArrays, FormGroups, Strings, any Iterable.

In my opinion null or empty string have a different meaning from an empty collection. When I'm validating a field I want to make sure the field contains a value different from null, or in empty string (whish I consider a special case of absence of value) On the other hand When I'm handling collections I want to make sure they have at least 1 item or I could check the collection is not null no matter if it has items or not, or I can check both, not null and with items. But that is something that needs to be explicitly validated because that all depends on the context and a particular requirement.

Merging both concepts in the same validator will not allow to satisfy all the possible scenarios. On the other way having them separated allows you to use one or both validators of they are needed.

Merging both concepts in the same validator will not allow to satisfy all the possible scenarios. On the other way having them separated allows you to use one or both validators of they are needed.

@joanpablo

Thanks for responding. That makes perfect sense. I can also envision the possibility that this seemingly small and harmless change may well break existing user code in unexpected ways. That said, it would be nice if the code comment can reflect this more precisely.

One of the problems with 'minLength(1)' is that it won't work for numbers (which is, BTW, very common in ReactiveTextFields). Also, in a l10n setting, if we changed to some sort of combination of required and minLength, we'd have to provide yet another validation message with the key ValidationMessage.minLength in addition to ValidationMessage.required.

Alternatively, given that RequiredValidator is what it is, can we have one NonEmptyValidator which, for all intents and purposes, behaves just like RequiredValidator and emit error messages using the same key ValidationMessage.required, and handles empty Iterables, Maps and Sets?

Currently this is what we've done in our own repo. Could be helpful for others as well.

Hi @laogao,

Thanks for the answer.
Would you mind giving me an example of the numbers validation issues that your were referring in your comments and how it relates with the minLength validator?

I also agree that we should update the comments on the required validator. So I would encourage you to create a PR that updates the comment on that validator and create a new validator that will serves as an alias to the Validators.minLength(1) called Validators.isNotEmpty. Does that sound good to you?

Would you mind giving me an example of the numbers validation issues that your were referring in your comments and how it relates with the minLength validator?

Hi @joanpablo,

No problem. The issue I had with minLength(1) is that if the ReactiveTextField's formControl has a type of num, when the captured value is, say, 42, which is a valid num, when it gets touched, the validation is expected to pass (if our intention is to have minLength cover the base of required and achieve more), yet it fails with the following message: {minLength: {requiredLength: 1, actualLength: 0}}.

(If you are curious, we had ControlValueAccessor in place to covert between model values and view values. So the underline data can be of any type.)

I know this particular case is probably not what minLength intended to cover. And I don't think it should, as it might cause other issues down the road.

As things stand, I'll have to specify both required and minLength(1) in order to archieve my original goal. This is more than simply dealing with one additional validator, it also means I'll be managing another set of i18n/l10n messages. I'd much prefer "re-using" the ValidatorMessage.required key. It could read: "required to be non-empty" and this feels quite natural to me.

Next I'll try to prepare a PR to fully reflect my thinking on this.

Cheers.

I'd much prefer "re-using" the ValidatorMessage.required key. It could read: "required to be non-empty" and this feels quite natural to me.

After some thought, it probably makes sense to leave ValidatorMessage.required as is, and create a separate ValidatorMessage.requiredNonEmpty. PR underway.

Hi @laogao

The issue I had with minLength(1) is that if the ReactiveTextField's formControl has a type of num, when the captured value is, say, 42, which is a valid num, when it gets touched, the validation is expected to pass

The Validators.required is intended to validate a non-null or empty String. On the other hand, the Validators.minLenght is designed to validate Iterables (List, Sets, etc), FormArry, FormGroup, String (as a collection of characters), and we might add Map (this one is not currently supported but it can be added as an improvement).

Validators.minLenght is not supposed to be used with num, I still don't get an example of why you would like to use it.

If you have a FormControl of type num you might want to just use ValidatorMessage.required, if you have a collection, then you might want to use ValidatorMessage.minLenght(1) to ensure there should be at least one element or you might want to use both ValidatorMessage.minLenght(1) and ValidatorMessage.required in the case of a collection that can also be null (this is less common since mostly all the times you shall create the FormControl with a default empty collection and not a null value).

That said, I'm not sure why to create another validator that will do exactly the same as ValidatorMessage.minLenght(1) but with a different name (this validator is for iterables, and kind of maps objects only)

Hi @joanpablo

Validators.minLenght is not supposed to be used with num, I still don't get an example of why you would like to use it.

To be clear, using minLength for num was never my intention. Let's rewind a little bit.

Where I started (simplified version)

• a survey question represented by aReactiveTextField for a value of type String.
• a survey question represented by aReactiveTextField for a value of type num.
• a multi-select question backed by another FormArray of checkboxes for a value of type Iterable<String>.
• all quesions can be specified as "required", which means the enduser has to provide some "non-empty" answer.
• if a question is "required", a proper validator has to be wired to that question.
• ideally this can be done while I build the whole survey (with multiple quesions of various types), with the validation as extra "decoration" (if a quesion is marked "required", a "required" validator is attached to it).

What I intend the validator to do?

• check if all "required" questions are answered (each have a "non-empty" value).
• if not, emit an validation error with a meaningful key.

What happens if Validators.required is used?

• for the String quesion, user has to input a non-empty string, otherwise the question is considered "unanswered" (this is good).
• for the num quesion, user has to input a number, otherwise the question is considered "unanswered" (this is good).
• for the multi-select question, user can leave it as is (a.k.a. []), and the question is considered "answered" (this escapes the intended validation, which is not good).

What happens if Validators.minLength is used instead to validate "required" (as suggested earlier)?

You can achieve that with the current validator ValidationMessage.minLength. You can just define ValidationMessage.minLength(1) to specify there should be at least 1 item in the Iterable. This validator also takes into account FormArrays, FormGroups, Strings, any Iterable.

• for the String quesion, user has to input a non-empty string, otherwise the question is considered "unanswered" (this is good).
• for the num quesion, no matter what the input, as long as it's a num, the question is considered "unanswered" (as it would fail the validation due to a length of 0, which is not good).
• for the multi-select question, user has to check at least one checkbox (this is good).

What happens if the proposed Validators.requiredNonEmpty is used?

• for the String quesion, user has to input a non-empty string, otherwise the question is considered "unanswered" (this is good).
• for the num quesion, user has to input a number, otherwise the question is considered "unanswered" (this is good).
• for the multi-select question, user has to check at least one checkbox (this is good).

Hope this can finally make myself clear.

I could, indeed, pick a different "required" validator for a different question based on its value type, but this would break a lot of code and is rather painful to maintain for my use case. I could also specify my own validator without touching reactive_forms's "pre-packaged" validators (which is currently the approach I've taken).

All things considered, a RequiredNonEmpty validator could be of some use for others, like myself, who are building online forms on top of reactive_forms. And that is this issue/PR all about. As to whether this particular change is a useful (sensible) addition to reactive_forms, I guess ultimately it's your call.

Cheers.

Thanks a lot, @laogao for clarifying your use case, now I understand your needs, and I really appreciate the time you took to explain it. I will give it another shot to the PR and think about it. IMO I would've used different Validators depending on the type of your questions, but as you explained that seems to add complexity to your code. Thanks a lot for your support.

Another thing I would like to know:

You mentioned that using different validators depending on the type of question is complex, is that complexity due to Reactive Forms, or it is intrinsic to your project? Is there any other thing (besides this new validator you are proposing) that Reactive Forms could change or do better in order to make you use different validators depending on the question a bit easier? Thanks

You mentioned that using different validators depending on the type of question is complex, is that complexity due to Reactive Forms, or it is intrinsic to your project? Is there any other thing (besides this new validator you are proposing) that Reactive Forms could change or do better in order to make you use different validators depending on the question a bit easier?

Hi @joanpablo appreciate your understanding.

The problem lies in the fact that we model our online forms in terms of questions (which are in turn built on top of various reactive forms widgets). Each question is of a different type (text, dropdown, radiogroup, checkbox, rating, ranking, image, file, etc.) and can have different attributes. Some of these attibutes are common, such as "isRequired".

To build the online form, we are given a spec file written in a "language" business ppl can understand and maintain. The first thing we do is mapping each question into respective reactive forms widgets according to its type. (I hope you would agree that the "type" in this context captures mainly "which form widgets to use" rather than the type of its underlying value.)

Our main focus at this stage is the widget tree that contains all questions business specified, renders properly, and can respond to user interaction. Validation is a separate concern, and usually comes to the main scene at a later stage: it doesn't normally affect how the questions/widgets are rendered. To deal with validation requires a different mind set. (And I think that might also be the reason you chose to put validators in their own package, and have type signatures of dynamics not Ts, rather than littering the logic inside form widget themselves.)

Now that user can see the questions and provide answers, we need to make sure the answers they provided are valid, before submitting the form to the backend. From business ppl's perspective, all they concerned about are "questions", not "widgets". So it makes perfect business sense for questions to "require" a meaningful answer, whatever that actually means. If no meaningful answer is provided, user would be prompted with the same message along the lines of "missing meaningful answer", no matter the answer need to be a String, a num, or a List of things. That's why a RequiredNonEmpty validator feels necessary and natural to me.

Hope this all makes sense to you. Cheers.

@joanpablo

Congratulations on the 15.0.0 release!

I noticed there was a change of style for validators. Pushed additional changes to match the new style. Hope you are still interested in this PR, namely #380 . Cheers.

Hi @laogao
Thanks! I Hope you are doing well.

Did you see the comments I left to you in the PR?

The one that I particularly concerned is using that validator for numbers, it really confuses me semantically speaking.

Did you see the comments I left to you in the PR?

The one that I particularly concerned is using that validator for numbers, it really confuses me semantically speaking.

@joanpablo

Thanks for your time and comments. As I explained earlier, specifically choose to use minLength on numbers was never my intention.

The idea was simple and straight-forward: a built-in common validator that can require the value to be non-null and non-emtpy, regardless of the type.

Currently we have the following built-in validators, and neither of them fulfills the need in its entirety:

• required does not catch empty lists / maps;
• minLength does not work for numbers;

I guess the confusion might come from the fact that initially you suggested to use minLength or an alias to minLength and call it a day, where a common validator that requires the value to be non-null and non-empty (at the same time) is all I asked for.

I opened the issue / PR with the thinking that maybe we could improve on the current required to handle iterables and maps. Turns out this would confuse or break existing expectations where empty lists or maps are regarded as valid values. That's why I ended up with a separate requiredNonEmpty in the hope that the new name speaks for itself: the value is required (non-null) and should be non-empty. In the same vein as requiredTrue: the value is required (non-null) and should be true.

Bottom line: I can provide my custom validator in my project, for sure (and thanks for keeping the extension point open). But I do think this common validator is useful for other reactive_forms users out there, too.

Cheers.