refactor(oneOf validator): Refactor oneOf validator

[zakhenry]

... to be less tied to ngx-subform implementation. Restructured errors for oneOf behaviour to be
explicitly named rather than implicit by array index location

[zakhenry]

This is still WIP, opened for discussion

[maxime1992]

I think there are good ideas and indeed we can push the oneOf a bit further but I'm not really keen on having to call oneOf as many times as we've got polymorphic properties that needs to be checked.

[maxime1992]

Why do you want to be able to change the error key?

[maxime1992]

Oh nvm I just saw why:

NgxSubFormValidators.oneOf(['allianceRebel', 'allianceImperial'], 'oneOfAllianceType')

Few things here, the key should probably be at the beginning (just a preference) but something that might lead to weird results:

When using 2 oneOf (or more), as the key is optional it would override the previous result if someone forgets to pass it.

I think it'd probably be better to have the definition of oneOf to be :

interface ControlInterface {
  randomKey: any;
  polyTypeA: string;
  polyTypeB: string;
}

interface FormInterface {
  subTypeA1: string;
  subTypeA2: string;
  subTypeB1: string;
  subTypeB2: string;
}

export function oneOf<ControlInterfaceType, FormInterfaceType>(
  keys: Partial<Record<keyof ControlInterface, FormInterface>>
) {
  // ...
}

oneOf<ControlInterface, FormInterface>({
  polyTypeA: ['subTypeA1', 'subTypeA2'],
  polyTypeB: ['subTypeB1', 'subTypeB2'],
});

This way, we only need to use oneOf once, it'll loop over the properties of the form once too (not X times where X is the number of times you call oneOf)

Also, in the above we'd be using as key what's defined in the original object and mapping that to keys of the sub form

The above doesn't fully work though...

I get Type 'string[]' is not assignable to type 'FormInterface' but that should be doable :)

[zakhenry]

The intent here, and what I’m optimising for is the common case. It strikes me as incredibly rare that you’d want to have multiple oneOfs in a single form, so I think we should not be making the api awkward for that one case.

I was also struggling with the problem of the error clobbering if you forgot to set the key name for differentiation, but I think you’ve got a great idea with having the argument be an object. However, I think we should support both options simultaneously, again to avoid an awkward interface for the extremely rare case. This could be handled by just checking if the first arg is array or object

[zakhenry]

Another possibly simpler option is to have NgxSubFormValidators.oneOf and NgxSubFormValidators.oneOfMultiple

[maxime1992]

Doesn't take into account if multiple properties are defined.

From the example above, if assassinDroid AND medicalDroid are defined there should be an error because we don't know which one to pick up.

That error should be optional when I think about it, and turned on by default.
But if someone prefers not to reset to null a value when the component is destroyed to restore previous values if the user come back to it, then he might turn it off. In that case, if he has a select to choose a given type, within the remap function he might just pick the corresponding value, according to the select so he'd be fine.

But I believe that by default we should have an error if more than one value is defined with a oneOf condition

[zakhenry]

You mentioned this in the other PR but I’m not getting what you mean? This code works and passes the unit tests?

[zakhenry]

From the example above, if assassinDroid AND medicalDroid are defined, notNullKeys.length is 2, not 1; there will be an error

[zakhenry]

One other thought I had - is this even a good idea to have in this package?

I’m wondering if we should create a NgxFormGroupValidators package? It really feels like a distinct package and I can already think of a few cases we would want already:

• oneOf (polymorphism)
• equals (password creation)
• greaterThan (min/max fields)
• ordered (drawing tool profiles)

[maxime1992]

discussed offline with @zakhenry:

• can't really do a oneOf pattern without ngx sub form anyway 🤷
• easy to make a breaking change later on if we move that to a separate repo

So all good to have that on ngx sub form 👍