ngneat / forms-manager

🦄 The Foundation for Proper Form Management in Angular
https://www.netbasal.com
MIT License
518 stars 29 forks source link

Improve Typings #2

Closed NetanelBasal closed 4 years ago

NetanelBasal commented 4 years ago

Currently, when we use the path parameter in the selectValue, and selectControl methods, we need to pass a generic that indicates what should be the type of the value property:

selectValue<string>('login', 'name');

This isn't the end of the world, but we can employ the technique used in this question to infer it automatically. So the expected usage should be:

selectValue('login', 'name');
selectValue('group', ['some', 'nested', 'control']);
Coly010 commented 4 years ago

I can take this on tomorrow

Coly010 commented 4 years ago

I'm having some issues trying to actually implement this with the current set up of how the path is resolved, and with the technique referenced in the issue.

It will return a flattened list of all keys in the FormState, however, selectValue's path argument allows for . to separate further nesting.

If we have a FormState of:

export interface AppForms = {
  onboarding: {
    name: string;
    age: number;
    address: {
     street: string;
     city: string;
     country: string;
   }
  }
}

and want to get the city value we would use selectValue('onboarding', 'address.city') which would not match a type using the technique above as the available types would be:

'onboarding' | 'name' | 'age' | 'address' | 'street' | 'city' | 'country'

Next, having a flattened list such as this will prevent us from inferring the type of the path argument as we cannot do FormState['city'] it would need to be FormState['onboarding']['address']['city']

The technique also raises an interesting dilemma where TypeScript currently only supports recursively mapping a Type to 50 levels of depth. Anything beyond this will throw an error.

This also raises an interesting question about whether all our Type information should live in one App-specific Interface, or rather, should it allow multiple forms to use their own Types that do not have to be declared at app level.

My thinking here is for people using Nx, where their form may live in a lib that can be pulled into any app, and also for third party libs that may setup forms that the app can then query from.

This issue of inferring the type certainly seems to be a lot more complex than I had originally believed it would be. I'm not sure if I am simply misunderstanding something or if I am overthinking it, but I think that for the time being, having to manually state what type is expected to be returned may be the only solution.

NetanelBasal commented 4 years ago

You probably missed the example in the first comment. We need to change the dot notation to an array:

selectValue('group', ['some', 'nested', 'control']);

We can start with three levels of nesting.

I'm pretty sure they can use Declaration Merging as I describe in this article: https://netbasal.com/better-code-organization-with-angular-di-multi-option-31f691918655

Coly010 commented 4 years ago

I'll give this another attempt later. Using the array notation for nested levels should hopefully help. I'll let you know how I get on with it.

Coly010 commented 4 years ago

I have still failed to do this via recursion, as it would mean flattening a Type Array, which keeps causing TypeScript to complain.

However, I have achieved this with the following:

export type NestedControlKey<
  A,
  T extends keyof A = keyof A,
  R extends keyof A[T] = keyof A[T],
  S = R extends keyof A[T] ? (A[T][R] extends object ? keyof A[T][R] : never) : never,
  U = S extends keyof A[T][R] ? (A[T][R][S] extends object ? keyof A[T][R][S] : never) : never
> = [R, S?, U?];

I'll be the first to admit it's pretty hairy.

The other problem arising is how do we then infer the Return Type of this. I achieved this with the following:

export type NestedControlType<
  A,
  R extends keyof A,
  T extends NestedControlKey<A, R>
> = T[2] extends null
  ? T[1] extends null
    ? A[R][T[0]]
    : A[R][T[0]][T[1]]
  : A[R][T[0]][T[1]][T[2]];

Again, pretty hairy.

However, it does work as can be shown in the example below:

class Something {
  a = {
    b: {
      c: {
        d: 'string',
        e: 2,
      },
    },
  };
  h = {
    num: 1,
  };
}

function myFunc<T extends keyof Something, R extends NestedControlKey<Something, T>>(
  form: T,
  path: R
): NestedControlType<Something, T, R> {
  return undefined;
}

const myValue = myFunc('h', ['num']);
const mySecondValue = myFunc('a', ['b', 'c', 'd']);

TypeScript can infer the types correctly : image

image

It also provides intelligent suggestions when completing the array of values:

image

@NetanelBasal If you have any thoughts, improvements etc, please let me know. I spent a long time trying to get the recursion method to work but the close I got was this a result that would require the api to look like this:

selectValue('a', ['b', ['c', ['d']]]);

As I could not flatten the array after each recursive call popped off the stack.

NetanelBasal commented 4 years ago

Yes, I see what you mean. I need to investigate it further. Let's leave it as is for now. Thanks for the work. I appreciate it.

Btw, what do you think about this issue?