fix: handle 'any' values properly in 'patchValue'

[rafaelss95]

PR Checklist

Please check if your PR fulfills the following requirements:

• The commit message follows our guidelines: CONTRIBUTING.md#commit
• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

[X] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Other... Please describe:

What is the current behavior?

patchValue doesn't handle any properly.

Issue Number: Fixes #58

What is the new behavior?

patchValue handle any properly.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

[itayod]

@rafaelss95 what about taking the implementation from here?

[rafaelss95]

@rafaelss95 what about taking the implementation from here?

Hmm, let me see... doesn't it would be an overkill? I mean, Angular forms doesn't support Set, Map, only literal objects and arrays. Do you consider the proposed implementation in this PR fragile?

[itayod]

@rafaelss95 what about taking the implementation from here?

Hmm, let me see... doesn't it would be an overkill? I mean, Angular forms doesn't support Set, Map, only literal objects and arrays. Do you consider the proposed implementation in this PR fragile?

Sure, I didn't mean "copy-paste" all of it, just the relevant parts... you can skip the Set / Map part

[itayod]

Also, your fix broke some tests, please notice that

[rafaelss95]

Also, your fix broke some tests, please notice that

I just noticed that and I may have found another bug (not really related by these changes) :(

Look at this (e is typed as any, but it's being recognized as FormArray and thus it isn't accepted in the creation of a FormGroup as a FormControl):

---

Edit: I found the bug.

The ControlsOf type

reactive-forms/projects/ngneat/reactive-forms/src/lib/types.ts

Lines 141 to 143 in ee41c40

	export type ControlsOf<T extends Object, TOverrides extends Partial<AbstractControlsOf<T>> = {}> = {
	[key in keyof T]: unknown extends TOverrides[key] ? ControlOf<T[key]> : TOverrides[key];
	};

... recognizes any values as to be FormArrays. Let me try to fix that in another PR.

[itayod]

@rafaelss95 I see that, would you like to create another PR that fixes that?

[rafaelss95]

@rafaelss95 I see that, would you like to create another PR that fixes that?

Done. Checkout it out in #60.

[rafaelss95]

Hey @itayod could you review #60? I'm waiting its review and merge to procceed the working here.

[rafaelss95]

Sure, I didn't mean "copy-paste" all of it, just the relevant parts... you can skip the Set / Map part

@itayod Removing the unneeded parts of it (ReadonlySet, Set, ReadonlyMap, Map, Function), we would have this:

export type PartialDeep<T> = T extends Primitive
  ? Partial<T>
- : T extends Map<infer KeyType, infer ValueType>
- ? PartialMapDeep<KeyType, ValueType>
- : T extends Set<infer ItemType>
- ? PartialSetDeep<ItemType>
- : T extends ReadonlyMap<infer KeyType, infer ValueType>
- ? PartialReadonlyMapDeep<KeyType, ValueType>
- : T extends ReadonlySet<infer ItemType>
- ? PartialReadonlySetDeep<ItemType>
- : T extends ((...arguments: any[]) => unknown)
- ? T | undefined
  : T extends object
  ? PartialObjectDeep<T>
  : unknown;

type PartialObjectDeep<ObjectType extends object> = {
  [KeyType in keyof ObjectType]?: PartialDeep<ObjectType[KeyType]>
};

... which produces exactly the same result as the one in this PR:

export type DeepPartial<T> = T extends object ? { [K in keyof T]?: DeepPartial<T[K]> } : T;

[itayod]

Sure, I didn't mean "copy-paste" all of it, just the relevant parts... you can skip the Set / Map part

@itayod Removing the unneeded parts of it (ReadonlySet, Set, ReadonlyMap, Map, Function), we would have this:

export type PartialDeep<T> = T extends Primitive
  ? Partial<T>
- : T extends Map<infer KeyType, infer ValueType>
- ? PartialMapDeep<KeyType, ValueType>
- : T extends Set<infer ItemType>
- ? PartialSetDeep<ItemType>
- : T extends ReadonlyMap<infer KeyType, infer ValueType>
- ? PartialReadonlyMapDeep<KeyType, ValueType>
- : T extends ReadonlySet<infer ItemType>
- ? PartialReadonlySetDeep<ItemType>
- : T extends ((...arguments: any[]) => unknown)
- ? T | undefined
  : T extends object
  ? PartialObjectDeep<T>
  : unknown;

type PartialObjectDeep<ObjectType extends object> = {
  [KeyType in keyof ObjectType]?: PartialDeep<ObjectType[KeyType]>
};

... which produces exactly the same result as the one in this PR:

export type DeepPartial<T> = T extends object ? { [K in keyof T]?: DeepPartial<T[K]> } : T;

there is still T extends Primitive though I can't think of any good use case for it I think we better add that too.
Let's go for this:

 T extends object ? {
  [P in keyof T]?: DeepPartial<T[P]>;
} : Partial<T>;

[rafaelss95]

there is still T extends Primitive though I can't think of any good use case for it I think we better add that too.
Let's go for this:

 T extends object ? {
  [P in keyof T]?: DeepPartial<T[P]>;
} : Partial<T>;

As Partial is a utility to be used for object, the use of Partial<anything except object> in the else path would be irrelevant as it would be to handle primitives, as you said and any, for example:

Partial<number> = number or Partial<any> = any.

Besides this, this change makes the build fail:

Anyway, to move forward with this PR, I've changed it accordingly. I reverted to the previous implementation.

[itayod]

do we have any reference of why this is failing now?

[itayod]

@rafaelss95

[rafaelss95]

Hmm, I didn't go deep but I faced this typing error:

Let me know if you have a better solution.

[itayod]

sure, let's try to find out why might be typescript's bug if so let's reference the issue

[rafaelss95]

I don't think it's a Typescript bug, it happens because value in this context is just typed as Obj ({ [key: string]: any }), however patchValue expects an Observable<DeepPartial<ControlsValue>> or DeepPartial<ControlsValue> and Obj isn't assignable to ControlsValue.

You can solve this problem or using any (as you already did in (

reactive-forms/projects/ngneat/reactive-forms/src/lib/formGroup.ts

Line 280 in 2a018eb

return super.getError(errorCode as any, path) as E[K] | null;

)) or by calling super.patchValue directly.

[itayod]

your explanation doesn't make sense at all, if that's true then how come it worked before? since you didn't change patchValue signature...

[rafaelss95]

Well, I didn't change patchValue signature directly, but indirectly. Anyway, as I had some more free time today I was able to fix it by changing the conditional in DeepPartial. Let me know if we can finally move on with this. Regards.

[itayod]

@rafaelss95 LGTM