ngx-sub-form icon indicating copy to clipboard operation
ngx-sub-form copied to clipboard

`remapFromFormGroup` shouldn't allow extra props to be returned

Open maxime1992 opened this issue 4 years ago • 2 comments

When creating a sub form we are strict and expect no missing properties, nor additional ones. This lets us behind the scenes use setValue instead of patchValue which wouldn't be really safe as we could end up patching something completely unrelated to form without noticing.

When using the remap functionality, we say that:

  • Incoming value of type ControlInterface should be transformed into an "internal" value that is of type FormInterface
  • The output value must be of type ControlInterface as well, driven by the current form value (of type FormInterface)

The issue is the following: TypeScript uses structural typing instead of nominal typing which means that extra properties are not an issue. So when the form has some internal state, for example the current value of a select when working with polymorphic forms, it's totally fine to just return the form value. Even though it contains and additional key/value.

Coming from a form, this could easily lead to runtime issues as the server could perform checks and make sure that no extra properties are here, etc.

We might as well be strict and enforce this ourselves by doing a little trick.

maxime1992 avatar Apr 06 '20 16:04 maxime1992

I did give a go at this... And here's the best I could achieve:

type Impossible<K extends keyof any> = {
  [P in K]: never;
};

export type NoExtraProperties<T, U  = T> = U extends Array<infer V>
  ? NoExtraProperties<V>[]
  : U & Impossible<Exclude<keyof U, keyof T>>;

It gives us quite nice results as long as the types are nearly the same (U extends T....). If that's not the case, it's all broken.

One thread/comment has a good summary about all that: https://github.com/microsoft/TypeScript/issues/12936#issuecomment-595140797

So we can assume this one is blocked by a 3rd party unless someone has a brilliant idea that I missed :)

maxime1992 avatar Apr 07 '20 15:04 maxime1992

:point_up: the above didn't work it always had edge cases not working, there's a typescript issue that we can track though which might make this doable at compile time: https://github.com/microsoft/TypeScript/issues/12936

maxime1992 avatar Apr 22 '20 14:04 maxime1992