No hook or observable member available to get latest form value after component initialised

[owen26]

There is currently onFormUpdate() hook and formGroup.valuechanges observable, but they are not called when the component gets the value from external after finish initialization.

There are indeed workarounds exist. But getting the latest form value after form initialization is still a legit scenario that is useful for this library.

[maxime1992]

Indeed there's no way for now to be aware when the form is patched by the parent.

It would be nice to have a proper hook or observable for that but the current workaround is to override the writeValue hook.

[cbleu]

I have the same type of trouble with the lib: to refresh sub-components when data are updated from an external source after the initialization (from a Rest api).
Maxime, can you explain a little bit more what is your proposal workaround with the writeValue hook and how to use it ?

[maxime1992]

Sure thing @cbleu

When using ControlValueAccessor (which is what ngx-sub-form is doing behind the scenes) when the parent update the value of a FormControl, Angular will call the writeValue hook on the children (the one implementing ControlValueAccessor).

That part is completely hidden and transparent when using ngx-sub-form, but as there's no hook (yet) to be warned when the value changes (patched by the parent) you can just hook yourself onto writeValue.

Something along the following lines:

// ...
public writeValue(obj) {
  super.writeValue(obj);

  // do what you want here
}
// ...

Is that enough for you now @cbleu? Hopefully it'll be useful until a proper feature/hook lands =)

[maxime1992]

@zakhenry I've encountered that one quite a few times and it makes the component really verbose.

Also to clarify the usage, in my case I'm passing an ID to a sub form and that sub form does access the store to retrieve a resource. When the ID changes locally (selection local to that component) it's all good. But when the ID changes upstream (from the parent) it's not fetching the new resource (at least not without the workaround).

I think it's a legitimate use case, what do you think?

If you agree, can you think of a good name for:

• a synchronous hook
• an observable that we might expose for dealing with streams in case the input might change a lot and the consumer wants to throttle or others

For the hook, we already have onFormUpdate =/
Should we make a breaking change and rename it to onLocalFormUpdate?
And the new one could be onGlobalFormUpdate?

Or we let the current one and bundle that in the next breaking change otherwise

[zakhenry]

Yea I think having a hook is useful - prob as simple as onWriteValue as that is explicit about what is happening

[maxime1992]

I'm fine with onWriteValue but I'd like something that combine both otherwise we'll end up with code duplication.

In my example above, I want to refresh the resource whenever the value as changed from the parent or locally. With hooks only I'd have to implement the fetch of the resource in both hooks.

Plus, as rxjs is the default for fetching data in most cases (ngrx, http calls, etc) I think it'd be nice to have an observable that we can subscribe to. The data flow would be much nicer IMO.

If we've got only a synchronous hook it means we'd have to subscribe there 😱
(to fetch data at least but could be nice for notifications or synchronous things)

What do you think of:

• a method onFormUpdate (already here)

• a method onWriteValue (to be added)

• a method onAnyFormUpdate (to be added)

• [maybe] an observable formUpdate$ (to be added, for consistency?)

• an observable writeValue$ (to be added)

• an observable anyFormUpdate$ (to be added)

[zakhenry]

Honestly, I think that is way overkill, and having a thinner interface at the cost of downstream verbosity is preferable.

[maxime1992]

Here's a concrete example of the boilerplate required for this to work today:

interface ResourceForm {
  resourceId: string;
}

@Component({
  selector: 'resource-control',
  templateUrl: './resource-control.component.html',
  styleUrls: ['./resource-control.component.scss'],
  changeDetection: ChangeDetectionStrategy.OnPush,
  providers: subformComponentProviders(ResourceControlComponent),
})
export class ResourceControlComponent extends NgxSubFormRemapComponent<
  string,
  ResourceForm
> {
  private resourceIdFromParent$: Subject<string | null> = new Subject();
  public resourceId$: Observable<string> = merge(
    this.formGroupControls.resourceId.valueChanges,
    this.resourceIdFromParent$
  );

  public resource$: Observable<Resource | undefined> = this.resourceId$.pipe(
    switchMap((resourceId: string) =>
      this.resourceFacade.getResource(resourceId)
    )
  );

  constructor(private resourceFacade: ResourceFacade) {}

  public writeValue(id: string | null): void {
    super.writeValue(id);

    this.resourceIdFromParent$.next(id);
  }

  // skipped everything related to ngx-sub-form on purpose to keep
  // the verbose part I'm trying to get rid of only
}

The way I think it should be:

interface ResourceForm {
  resourceId: string;
}

@Component({
  selector: 'resource-control',
  templateUrl: './resource-control.component.html',
  styleUrls: ['./resource-control.component.scss'],
  changeDetection: ChangeDetectionStrategy.OnPush,
  providers: subformComponentProviders(ResourceControlComponent),
})
export class ResourceControlComponent extends NgxSubFormRemapComponent<
  string,
  ResourceForm
> {
  public resource$: Observable<Resource | undefined> = this.anyFormUpdate$.pipe(
    switchMap(formValue =>
      this.resourceFacade.getResource(formValue.resourceId)
    )
  );

  constructor(private resourceFacade: ResourceFacade) {}

  // skipped everything related to ngx-sub-form on purpose to keep
  // the verbose part I'm trying to get rid of only
}

What would you do?

[zakhenry]

I think if that is the use case, then we implement just that. i.e. have a single new observable value that is public controlValue$ or similar.

[maxime1992]

I'm (nearly) fine with that.

I think it'll just be odd to leave the existing hook (onFormUpdate which is for local changes) without having another one to get updates whenever the form changes in any way. So after adding that observable, we should probably either:

• Add a synchronous hook to be warned when the form has changed (parent or local doesn't matter)
• Remove the existing one

Otherwise it's not really consistent to have sometimes observables, sometimes synchronous hooks

[zakhenry]

Yea, I think that is fair, it does seem that onFormUpdate was perhaps a mistake and should be deprecated?

[maxime1992]

I think onFormUpdate is a good idea but realistically... not that good as we're only able to call that method individually for every props that have changed 🤷‍♂️

CF implementation https://github.com/cloudnc/ngx-sub-form/blob/master/projects/ngx-sub-form/src/lib/ngx-sub-form.component.ts#L369-L370

I'm still unsure whether we should deprecate it or not.

[maxime1992]

Another example of why I'm tempted to deprecate #127

[ntziolis]

If the goal is to make sure that value changes of the formGroup always outputs the latest data why not:

• create a subject in the base class NgxSubForm that captures values on writeValue, say externalValue$
• reassign the valueChanges property via `formGroup.valueChanges = merge(externalValue$, formGroup.valueChanges)

This makes the changes available in the subform without interfering with the formGroups internal events / states etc.

This does not solve the issue of valueChanges not firing on individual controls on the subform, but I think having a trigger for the overall subform would already be extremely helpful.

I'm usually not the biggest fan of this overwriting approach, but in this case I feel it might be warranted as it would provide a transparent solution without the need for additional hooks etc.

[maxime1992]

@ntziolis the override would be an issue as if you update the internal form when the parent calls writeValue, you'll end up broadcasting that value up to the parent again while you just don't want to do that. If the parent updates the child, the child should handle that internally without telling the parent "I changed! Here's the same value". That's why we'd probably have to have a different observable

[maxime1992]

BTW here's a small update of the workaround as I just encountered a bug:

interface ResourceForm {
  resourceId: string;
}

@Component({
  selector: 'resource-control',
  templateUrl: './resource-control.component.html',
  styleUrls: ['./resource-control.component.scss'],
  changeDetection: ChangeDetectionStrategy.OnPush,
  providers: subformComponentProviders(ResourceControlComponent),
})
export class ResourceControlComponent extends NgxSubFormRemapComponent<
  string,
  ResourceForm
> {
  private resourceIdFromParent$: ReplaySubject<string | null> = new ReplaySubject();
  public resourceId$: Observable<string> = merge(
    this.formGroupControls.resourceId.valueChanges,
    this.resourceIdFromParent$
  ).pipe(shareReplay({ bufferSize: 1, refCount: true }));

  public resource$: Observable<Resource | undefined> = this.resourceId$.pipe(
    switchMap((resourceId: string) =>
      this.resourceFacade.getResource(resourceId)
    )
  ).pipe(shareReplay({ bufferSize: 1, refCount: true }));

  constructor(private resourceFacade: ResourceFacade) {}

  public writeValue(id: string | null): void {
    super.writeValue(id);

    this.resourceIdFromParent$.next(id);
  }

  // skipped everything related to ngx-sub-form on purpose to keep
  // the verbose part I'm trying to get rid of only
}

[ntziolis]

I had a longer conversation with a colleague and after thinking about it we actually like the fact that its not transparent and that one has to take action to react to external form changes seperately from internal changes.

But we would love to have some syntactic sugar :)

I was thinking something in the line of:

• provide an additional generic type to the base sub form class
• the additional type would have to be a subset of the FormInterface (and defaults {})
• then use this type to expose an object from the subform
  • with the properties of the additional type
  • each property would hold a subject
  • the observables would be fed via writeValue on the base sub-form pushing values into them

This would only be done for the properties on the additional type, hence no per impact if one doesn't need to access external value updates whicle providing easy access (using merge as above but without the need to handling the subjects).

[ntziolis]

Looking into implementing a sample of the above I realized its not feasible to go the route of using a third interface to reduce the keys as we do not have runtime access to the keys (its different for FormInterface as we actually an object with all keys defined Controls).

Given the above I see the following options:

• returning a structure with a subject for each key
• returning a single subject containing the complete transformed form value
• returning a single subject with no data that simply lets one know when an external update has happened

From the above I prefer option 2 as it would cause almost no performance impact (vs option 1), while likely covering most use cases without having to grab data after getting the signal (vs option 3).

In general, since the main use case for this is consuming it within the subform in a continuous manner I vote for returning a subject or an observable rather than just exposing a hook method as all that would do is move the work of implementing a subject from writeValue into a new method.

[maxime1992]

we actually like the fact that its not transparent and that one has to take action to react to external form changes seperately from internal changes.
But we would love to have some syntactic sugar :)

I fully agree here.

Re-reading my comment and taking into account the fact that onFormUpdate has been deprecated, I think we now only need 2 observables: writeValue$ and anyFormUpdate$ (both could be named differently but you get the point).

What do you think @ntziolis & @zakhenry?

[ntziolis]

@maxime1992 These two observables would work for us. in fact I like the name writeValue$ a lot as it is super clear what it does or where it comes. Any other name I can think of right now would need a lengthy explanation of when it gets fired and why.

In regards to anyformUpdate$:
I would probably call it formGroupValue$ as it resembles what formGroupValuesdoes only on formGroup level + observable which makes the name very descriptive.

[zakhenry]

I would suggest a third option too - controlValue$ - to be able to observe from inside the component what the current value of the control is as seen from outside (this is the combination of writevalue and mapped formvalue)

[maxime1992]

@zakhenry's I can't think of any use case right now, did you come across one or do you have one in mind?

Also, reading your comment I realise that I may not have been clear enough in my previous comments but about the writeValue$, I think it should always/only be the value after the remap function (transformToFormGroup) has run on the input. So maybe we could come up with a clearer name? I'm afraid writeValue$ might be a bit misleading but it may also be fine as we've got type safety... Any suggestion?

[ntziolis]

You are right, forgot about the remapping. Then maybe something like writeFormValue$, but I feel this is way less prescriptive than writeValue$. And you are right the typing prevents users from errors. So after thinking about it some more I’d still vote for writeValue$, but only because I cannot think of a better alternative. Damn naming is hard xD

[ntziolis]

Still 100% on board with the additional observables. However I just realized this won't fix what initially raised this issue for us:

@ntziolis the override would be an issue as if you update the internal form when the parent calls writeValue, you'll end up broadcasting that value up to the parent again while you just don't want to do that. If the parent updates the child, the child should handle that internally without telling the parent "I changed! Here's the same value". That's why we'd probably have to have a different observable

100% we do not want to create a circular data flow. With this in mind I relalized that there are edge cases when writeValue actually should emit events to the parent and that's when the value passed into writeValue is different from what is actually stored in the controls.

For example:

• When a reset() is triggered on the outer form and there are default values specified in the sub form
• The value inside the sub-form is no longer the same as the value that was passed into writeValue
• Hence in this case the sub form should emit to the outer form

The same is true when the sub-form is initialized with null or undefined and has default values (looking into the code changing it for reset would actually fix the form initialization case as well as from code perspective they are the same).

Why is this crucial for us:

• We have lots of interrelations between fields on different parts of the sub form tree
• With ngx-sub-forms we loose the ability to access the controls of the sub-form from the parent (no formGroup.get("subForm.controlOnSubForm')
• So there is no way to set validations / disable fields etc. from outside the sub-form
• We do not want to pass up and down relevant data, istead we use state mgmt for the form to be able to access the entire form data from within any sub-form
• We use state handling via akita (very happy with that btw. if you are using Redux or similar and the concepts feel strange in an angularworld have a look at akita)
• Akita takes a formGroup and attaches to the relevant hooks eg valueChanges after that the form is synced with the state

Right now that sync is broken as neither:

• initial default values show up in the store after form has been loaded
  • they do show up after any field is changed
• nor does a reset properly clear the form values

What do you think? Should this be dealt with in a separate issue since this is not about extending the api of ngx-sub-forms?

[ntziolis]

To address my last comment I was thinking something like this:
#152

Only created a PR to see if there are side effects as I cant run CI locally anymore (windows).

[ntziolis]

I have used patch package to monkey patch the changes I outlined above and I can confirm that the approach above does remove the state sync issues we have seen. Default values are now properly bubbled up after being set in the sub-form.

[maxime1992]

Discussed with @zakhenry and what we should do is:

• Have a BehaviorSubject (that we expose publicly as an Observable) and this BehaviorSubject would emit the FormGroup as soon as it's created (see How to have validatorFN for FormControl inside getFormControls() ? #82 and [DO NOT MERGE YET] fix(lib): access class props from get form controls #113 as the creation may move to ngOnInit)
• Have another BehaviorSubject exposed as an observable which would switchMap from the first one (the form group) and expose only the form group value + combine with the writeValue one to solve this issue (No hook or observable member available to get latest form value after component initialised #86)

[maxime1992]

🎉 This issue has been resolved in version 5.2.0-feat-rewrite.1 🎉

The release is available on:

• npm package (@feat-rewrite dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[zakhenry]

@maxime1992 is this issue actually fixed by the rewrite? I'm not sure how to get access to that observable? I think we need to expose this separately in the interface NgxSubForm

[maxime1992]

@zakhenry yes it's been solved.

You can now simply subscribe to form.formGroup.value and this will always be up to date 👍

Whereas previously if a value changed upstream because writeValue was called this observable was not emitting and we had to do all sorts of hacks to go around this.

You can easily test it by subscribing to form.formGroup.value and console log the result then change the value upstream by patching the parent form for example

[github-actions]

🎉 This issue has been resolved in version 6.0.0-feat-rewrite.5 🎉

The release is available on:

• npm package (@feat-rewrite dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[github-actions]

🎉 This issue has been resolved in version 5.2.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀