Error is not updated when FormArrayState has empty array value

[icepeng]

When FormArrayState has [] value, error is not updated with SetErrorsAction() or setErrors().

This is necessary because there are use-case of "Array must have one or more value".

If this issue is resolved, validate(required) would be extended to validate FormArrayState.

[MrWolfZ]

Just from looking at the code I cannot spot the reason this would happen. I'll have to have a more thorough look after work.

[MrWolfZ]

@icepeng I was missing some test cases for this, but otherwise setting errors on empty array state works just fine. Are you still encountering this issue? If so, could you provide some example code for it?

[icepeng]

@MrWolfZ This issue still appears for me. I will update minimal reproduction code tomorrow. Thank you.

[icepeng]

https://github.com/icepeng/ngrx-forms-issue
Created repo. There are two type of errors: when item.length > 5, or item.length === 0.
Too many items error is generated properly, but no item error is not generated.

[MrWolfZ]

Thanks for the reproduction. It helped me find the issue. The problem here is that the array state is defined to be enabled if any of its children is enabled. That means an array without children is always disabled. That in turn causes the error to be ignored since disabled controls cannot have errors set. The same goes for empty groups.

The obvious fix is to invert the condition, but I'll have a good long think about this one before I decide this. I'll update this thread once I've made a decision on how to handle this issue.

Also, the reason why my tests didn't show the issue is that there is an inconsistency between how an initial state is created and how a state is recomputed after a change. I'll fix this as well.

[icepeng]

Great. Could there be a workaround to make my project work before you make a decision?

[MrWolfZ]

Sure, sorry for not mentioning one. I assume your array is part of a larger group state like in your reproduction, right? In that case I would simply put the error on the containing group. This is of course a bit ugly and might not work if you have your form state split over multiple templates.

So in your reproduction repo you could use this code:

const atLeastOneItem = (myForm: MyForm) => {
  if (myForm.items.length > 0) {
    return {};
  }

  return {
    noItem: true,
  };
};

const atMostFiveItems = (myForm: MyForm) => {
  if (myForm.items.length <= 5) {
    return {};
  }

  return {
    tooMany: myForm.items.length,
  };
};

const validationFormGroupReducer = (state: FormGroupState<MyForm>, action: Action) => {
  state = formGroupReducer(state, action);
  state = validate([atLeastOneItem, atMostFiveItems], state);
  return updateGroup(state, {
    value: validate(required),
  });
};

However, I am already working on the proper fix. I will invert the condition for the isDisabled check. While this is a breaking API change it only affects the edge cases of empty groups or arrays. And as in your case here it should definitely be possible to validate the group/array in such cases.

[icepeng]

In my case, there is child component used like <items-form [itemState]="formState.controls.items"></items-form>. Because there are several type of forms that share items form. Moving error to each parent group would solve the problem, but it would produce a lot of duplication in code base.

Since it is not critical for me right now, I decided to wait until this issue is resolved. Not just waiting for your contribution, I am looking into repo deeper to find out how I can help this project. I found out that required, minLength, maxLength validators would work well with array. How do you think about it?

ps. Thank you for detailed reply!

[MrWolfZ]

You are right, those validation functions could easily be extended to apply to arrays as well. Since I am already working on the library right now I could add those cases. Or would you like to contribute a PR for this?

[icepeng]

I have no experience contributing a PR, except for fixing some docs.

It would be nice if I can send my first PR for this library, but I am worried about my lack of experience. I will ready for PR in this week. Then, please point out my mistakes.

[MrWolfZ]

Everybody starts without experience, so no worries ;)

I'll wait for your PR and then give you feedback.

[MrWolfZ]

I have just released version 2.2.0 which fixes this issue.

[icepeng]

I submitted PR yesterday #40