MrWolfZ/ngrx-forms

Struggles with composing functions

Zonnex opened this issue · 3 comments

Hi

When we're using various functions in the library, it's common for us to use multiple functions to update the state.
We have also created some helper functions to set userDefinedProperties like visible/invisible but they are not used for this example but to give more context.

For example:

state = setValue(state, 0);
state = enable(state);
state = markAsTouched(state);

I saw the other day that there is a compose function that could help reduce some noise, so i tried using it

state = compose(setValue(0), enable, markAsTouched)(state)

However, the type inference doesn't work now. I get an AbstractControlState<unknown>.

Do you know if there is another way to accomplish this or if typescript type inference isn't strong enough?

Thanks!

Hmm, yeah, type inference is a bit tricky. The following works correctly:

const state = compose(setValue(0), enable, markAsTouched)(createFormControlState('', 1));

state is correctly typed as FormControlState<number>. However, if we change this slightly it stops working:

let state = createFormControlState<number>('', 1);
state = compose(setValue(0), enable, markAsTouched)(state);

Here the state gets inferred from setValue, which accepts an AbstractControlState. However, we can tell compose what type we expect:

let state = createFormControlState<number>('', 1);
state = compose<FormControlState<number>>(setValue(0), enable, markAsTouched)(state);

This works, but it's a bit annoying that we have to manually specify the type. However, fortunately TypeScript allows referring to the type of a variable. That way we can slightly simplify the code and make it more robust against changes:

let state = createFormControlState<number>('', 1);
state = compose<typeof state>(setValue(0), enable, markAsTouched)(state);

I think while this is not perfect, it should be sufficient.

PS: be careful never to accidentally import compose from @ngrx/store, since while that one is arguably more accurately typed, it is much worse for type inference.

Thanks for the quick reply and thanks for the suggestions, they will help us :)

I agree, using compose<typeof state> is sufficient for most cases. There are some cases that can't be solved and that's fine. compose<typeof state>(setValue(0), markAsTouched, enable)(state); fails when state type is FormControlState<number | null>
because setValue(0) can't know that the value is going to be used for something that could be null. I could specify the type setValue<number | null>(0) but it's not that great.
For these cases, splitting it in 2 is not that bad:

state = setValue(state, 0);
state = compose<typeof state>(enable, markAsTouched)(state);

Yeah, that seems reasonable enough. I don't think I can "convince" the type inference to handle that case nicely ;)

I'll close the issue. If you have more questions feel free to continue writing here or open a new one.