Major architecture changes coming

[maxime1992]

Hello 👋

What's this all about?

I'd like to share a little update on ngx-sub-form on behalf of @zakhenry and myself as we've been pretty quiet for a while. We've had some work at CloudNC we needed to focus on and with the Covid-19 coming in between, we've all had to adapt. So if you've commented on an issue, made a PR or made any kind of interaction and haven't had any answer don't take it badly. We still care a lot about ngx-sub-form and we'll try to get back to you asap 😸.

Better inclusion of ngx-sub-form into our planning (at CloudNC)

@zakhenry and I have been trying to build a dependency graph of all the issues, order and prioritize them in order to start including some of them as part of our work time (during our sprints 🙌). We hope to tackle some tech debt on ngx-sub-form pretty soon 💪.

New architecture

I spent a whole day thinking about all the issues, how they connect to each others, how to solve one without adding unwanted behavior, etc. The following lead me to think that we need to rework on the architecture of the project and make major changes.

From what I've found, concepts will remain the sames but:

• Internal of the lib will be (nearly) fully rewritten to be more flexible and hopefully simpler ✨
• Public API would also have some changes but more like moving things around rather than the need to rewrite all your forms from the ground up! 😸

Changes

Let's start with a rough diagram of the new internal data flow. Please don't focus too much on this yet, I'll refer to it point by point explaining what's new and what it'll bring:

the image has a good quality, if it's not big enough open it in its own tab

Creating a sub/remap/root form component

First big change: Lets get rid of inheritance 👍.

Here's how a basic sub form would look like with the new API:

@Component({
  // all the usual
})
export class SubFormComponent {
  public subForm: SubForm = createSubForm<YourSubFormType>(this, {
    formControls: {
      prop1: new FormControl(),
      prop2: new FormControl(),
    },
  });
}

Related issue:

• Can I use injected dependencies in a hook? #160 & How to have validatorFN for FormControl inside getFormControls() ? #82 (would be fixed)
• RFC: refactor to one class with multiple implementable interfaces #144 (would be obsolete)
• Single field sub form interface #123 (would be easy to implement)
• Ngx*RootFormComponent should force to implement the remap methods if 2 interfaces are provided #133 (would come for free as the options are passed through an object which we can type correctly)

Stop using internally { emitEvent: false } every time we interact with the public form group

Internally the biggest issue we face is how to apply changes to our form group when required by the parent (through writeValue or even setDisabledState for example) without broadcasting those values straight up to the parent.

When the value of the model changes upstream, we want to put it into the form group, wait for the user to update the form, and for every subsequent changes broadcast the value up. In order to achieve this, we previously used { emitEvent: false }:

ngx-sub-form/projects/ngx-sub-form/src/lib/ngx-sub-form.component.ts

Lines 108 to 110 in 00aea19

	if (!!defaultValues) {
	this.formGroup.reset(defaultValues, { emitEvent: false });
	}

ngx-sub-form/projects/ngx-sub-form/src/lib/ngx-sub-form.component.ts

Lines 118 to 119 in 00aea19

	if (this.formGroup) {
	this.formGroup.updateValueAndValidity({ emitEvent: false });

ngx-sub-form/projects/ngx-sub-form/src/lib/ngx-sub-form.component.ts

Lines 245 to 250 in 00aea19

	this.formGroup.reset(
	// calling `reset` on a form with `null` throws an error but if nothing is passed
	// (undefined) it will reset all the form values to null (as expected)
	defaultValues === null ? undefined : defaultValues,
	{ emitEvent: false },
	);

ngx-sub-form/projects/ngx-sub-form/src/lib/ngx-sub-form.component.ts

Lines 271 to 273 in 00aea19

	this.formGroup.setValue(transformedValue, {
	emitEvent: false,
	});

ngx-sub-form/projects/ngx-sub-form/src/lib/ngx-sub-form.component.ts

Lines 422 to 426 in 00aea19

	if (shouldDisable) {
	this.formGroup.disable({ emitEvent: false });
	} else {
	this.formGroup.enable({ emitEvent: false });
	}

This way, the changes would be reflected on the form but our internal subscription to formGroup.valueChanges wouldn't be triggered straight away when writeValue is called.

Unfortunately, this led to some (annoying) issues:

• disabled state is not emitted, blocking external listeners to react to changes of the form's status #143 (would be fixed)
• No hook or observable member available to get latest form value after component initialised #86 (would be fixed)

How is the upgrade going to work?

We'll try to make the upgrade as smooth as possible by deprecating the old way of using ngx-sub-form through classes and expose the new functions once they're ready. This should let you do an incremental update if you've got a lot of components using ngx-sub-form. (and don't worry, we have ~120 components which extends Ngx*FormComponent so we'll have to go through that ourselves 😅!)

We'll create a next branch were we'll be able to make pre-releases on CI so that we can thoroughly test internally that everything is fine before publishing it for good 👍 and ask for some help from the community to see if everything is fine for everyone!

The first target will be to reach features parity. Once we reach that objective we should be able to cut a release (a feature release, not a breaking change 👌). From there, already quite a few issues should be solved and we should be able to move on to new issues.

How can you help?

• You've been using ngx-sub-form in your project(s)?
  (Please let us know here Who's using ngx-sub-form? #112 😸)

• You have ideas that we should take into account during the rewrite?

• You're keen to help for review once the PR is out?

• Have anything interesting to share?

Please comment directly on this issue so that we can all end up with a good way of working with our forms 😃!

Following is just the Excalidraw backup for the schema
ngx-sub-form-new-architecture.zip

[ntziolis]

First of ngx-sub-form has brought a massive boost in developer onboarding speed + productivity + code quality for us. Obviously I can only speak for my team, but the increases were so big that we wold be 100% fine with rewriting the hundreds of forms we have already written against the existing API, based on the advantages of the new approach.

I'm happy to participate in any way necessary (calls, implementation etc.) to get this off the ground quickly. My schedule is wide open for this (based in Europe).

Sidetrack:
From what I understand the changes are mainly around how a sub form handles changes and when those changes are emitted and that's great. I'm assuming it would not address things like #155 (and that's totally fine). I'm asking to understand if I should halt the work I'm doing to address those kind of issues.

[ntziolis]

This is going to be a long one, please bear with me. Since there is going to be some rearch I tried to think about what the benefits / goals are of ngx-sub-forms and what we (my team) have learned while using it. Then I reset my brain to try and see if there is a completely different approach, just to make sure we are not missing something. This is not meant to derail the main discussion and if you feel this issue is not the right place I'll open a new one.

First lets go though the key advantages of using ngx-sub-form:

• it makes it easy and DRY to create custom angular components that can plug into the reactive forms infrastructure.
• it provides standardisation for things like
  • structure
  • transformation
  • default values
• it provides typing of controls within a sub-form
• it provides easy + typed access for commonly used things (formControlNames etc.)

At the core it achieves this by implementing ControlValueAccessor, which is an elegant solution that enable custom components to "just work" with the reactive form directives.

This approach solves A LOT of problems and has served us well, but it does have one major caveat. It hides the controls of a sub-form from the parent. However without standardization of the structure of sub forms, there was simply no other way to achieve enabling the above.

Maybe you already know where I'm heading with this. By using ngx-sub-form the structure of sub forms is actually well defined. Which does open an avenue outside of leveraging ControlValueAccessor.

The main three pieces would be:

• Defined structure of a sub-form (same / similar as today)
• A custom directive e.g. [subFromGroup] that replaces [formGroup]
• Wrappers for FormGroup / ArrayForm

Custom directive e.g. [subFromGroup]

• The only job of the directive is to connect the FormGroup / FormArray wrapper instance with the sub-form-component and vice vera
• This results in the sub-form-components internal FromGroup instance being attached to the FromGroup instance of the parent, in fact it will behave 100% like creating a normal FormGroup without using ngx-sub-forms
• This automatically enables things like
  • markAllAsTouched / dirty
  • or even more advanced scenarios like changing validators from outside a sub-form
  • or any other use case that is currently not available due to reactive forms not knowing what controls are inside the sub form

"Wrappers" for FormGroup / ArrayForm

• Those would be used in the ´getFormControlsinstead of newFormControl`
• Most of the methods called on the wrapper would be handled by the base class FormGroup / ArrayForm
• Some methods / fields would NOT be handed through to the underlying FormGroup / ArrayForm instance e.g. valueChanges as we wanne transform the values provided by the sub-form before emitting them to the parent

By making the wrappers generic it would even be possible to have typings for the controls on the sub-form (not just the output value). Granted, this means exposing the FormModel from the sub form to the outside world. So this should be optional.

Today there is a lot of smartness in the SubFormComponent base class especially around handling the values. This is due to the fact that it had to bride between ControlValueAccessor and the FormGroupon the sub form. By taking out the necessity to bridge the logic in the class would likely become significantly less complex.

The downsides of the non ControlvalueAccessor approach that I can think of right now are:

• Custom directive instead of built in reactive forms directives
  • This might turn some people off that feel that is moving to far away from the standard.
  • That said when going the ngx-sub-form route you are already buying into a stack. SO this might not be
• User needs cannot just use new FormControl(null) inside the getFormControls method,
  • Instead they need to use new SubFromGroup() / new ArrayForm() when a property is representing a sub form
  • That's in line with standard reactive forms, but it was nice to always be able to blindly use new FormControl

Enough words here is some code to illustrate.

Directive

// 99% copied from angular source
@Directive({
  selector: '[subFormGroup]',
  providers: [formDirectiveProvider],
  host: { '(submit)': 'onSubmit($event)', '(reset)': 'onReset()' },
  exportAs: 'ngForm'
})
export class SubFormGroupDirective<TControl, TForm> extends FormGroupDirective implements OnInit {
  @Input('subFormGroup') form: SubFormGroup<TControl, TForm>;

  constructor(
    @Optional() @Self() @Inject(NG_VALIDATORS) validators: Array<Validator | ValidatorFn>,
    @Optional() @Self() @Inject(NG_ASYNC_VALIDATORS) asyncValidators: Array<AsyncValidator | AsyncValidatorFn>,
    private host: NgxSubFormComponent<TControl, TForm>
  ) {
    super(validators, asyncValidators);    
  }

  ngOnInit(){
    // this method would be new on the sub form base class
    this.host.initFormGroup(this.form);

    // we need access to the sub form component instance to handle things like transformations
    this.form.attachSubFormComponent(this.host)
  }
}

Wrapper

export class SubFormGroup<TControl, TForm> extends FormGroup {
  private subForm: NgxSubFormComponent<TControl, TForm>;

  constructor(
    validatorOrOpts?: ValidatorFn | ValidatorFn[] | AbstractControlOptions | null,
    asyncValidator?: AsyncValidatorFn | AsyncValidatorFn[] | null
  ) {    
    // initialize an empty form group at first
    // controls will be added when form group is injected into sub-form
    super({}, validatorOrOpts, asyncValidator);
  }
  
  // required to allow injection of the sub form component from outside after the Wrapper was initialized
  attachSubFormComponent(subForm: NgxSubFormComponent<TControl, TForm>){
     this.subFrom = subFrom;
  }
  
  // override impl samples
  get valueChanges() {
    // here we use the injected sub form component instance to handle transformation
    return super.valueChanges.pipe(
        map(value => this.subForm.transformFromFormGroup(value))
    );
  }

  setValue(value: { [key: string]: any }, options: { onlySelf?: boolean; emitEvent?: boolean } = {}) {
    super.setValue(this.subForm.transformToFormGroup(value), options);
  }
}

I'd be super keen to know what you think. I understand this would be a complete departure from how the lib works today.

[ntziolis]

First off sry for spamming this thread, but couldn't let this rest yesterday and spend all night getting this up and running based on latest v4 (as we have a ng8 project where we need it).
I did have to work through a bunch ts compiler issues, but once I worked through these data flow + advanced use cases like markAsTouched from parent worked pretty much right away.

This is with ControlValueAccessor and internal value changes handling completely ripped out of the SubFormComponent.

But (you knew it was coming):

• ngOnInit on the SubFormComponent gets called prior to ngOnInit on the directive which injects the FormGroup instance into the SubFormComponent
• This is a critical blocker as user should NOT have to place his init code in a custom method instead of ngOnInit which would really be the only way around this.
  • Even if that would be acceptable, this has ripple effects as it would happen outside of angular's lifecycle so other lifecycle methods would be called before the custom init method

At this point I thought this approach was a dead end and then it hit me.

Why not go even simpler:

• The approach below would not require a custom directive
• Since a SubFormComponent is well defined, we could simply enforce a simple input property e.g. subForm which holds the SubFormGroup instance
• At first sight this seems like a step backwards, but when thinking about it further all we want is a standardized way to handle sub forms + write as little repetitive code as possible
• in regards to DRY + ease of use, instead of having to remember to write:
  • providers: generateSubFormProviders(MySubFormComponent)
  • its now @Input() subForm : SubFormGroup
  • So the amount of non DRY code does not change, its just different
  • In addition
    • the subFrom input property can be enforced via inheritance (yes one still needs to remember to add @Input())
    • While adding @Input() can still be forgotten by the dev angular will provide a descriptive error message (identifying the component and missing input by name), while a missing generateSubFormProviders will cause a non descriptive error
• Another big advantage is that this would standard angular (in fact its their recommendation to handle forms on another component to use input props)

With this ins mind, what would be the role of ngx-sub-form:

• DRY + easy sub forms (same effort as before)
• standardisation (same as before):
  • structure
  • transformation (by wrapping FormGroup / FormArray)
  • default values
• typings of controls within a sub-form (same as before) as well as from the parent (new)
• easy + typed access for commonly used things (formControlNames etc.) (same as before)

But it would not be involved in value / observable handling at all (besides transformation inside the wrappers for FormGroup / FormArray).

Thoughts? Totally understand if this is not a direction you would consider.

[ntziolis]

Couple of lines of code are better than thousand words (basics are already there and have it up and running in our project): https://github.com/ntziolis/ngx-sub-form/tree/sub-form-controls-simpler

[maxime1992]

First lets go though the key advantages of using ngx-sub-form

Very good summary 👍

This approach solves A LOT of problems and has served us well, but it does have one major caveat. It hides the controls of a sub-form from the parent. However without standardization of the structure of sub forms, there was simply no other way to achieve enabling the above.

Correct. Unfortunately ngx-sub-form was created initially because of the lack of support for sub forms.

Maybe you already know where I'm heading with this

I have a little idea 😄!

The main three pieces would be:
Defined structure of a sub-form (same / similar as today)
A custom directive e.g. [subFromGroup] that replaces [formGroup]
Wrappers for FormGroup / ArrayForm

I'll answer point by point:

Custom directive e.g. [subFromGroup]

This is a very interesting point. But I suspect we may run into some expression have changed error as modifying the sub form (which would be attached to the parent) would trigger instant changes for the parent component. Not 100% sure though...

"Wrappers" for FormGroup / ArrayForm

Instead they need to use new SubFromGroup() / new ArrayForm() when a property is representing a sub form

One gap I've been trying to close into #147 is the fact that we're not sure at build time about how things get connected within the view. Unlike an input, there's no way to really type a control value accessor.... So we do not get any AoT safety. If we could guarantee that for sub forms that'd be 🤯. I know it's not exactly what you were referring to but I got inspired ha.

Today there is a lot of smartness in the SubFormComponent base class especially around handling the values

As long as this smartness is black boxed into the lib and it's easy to consume I'm pretty happy with it. Also, with the ongoing rewrite I think I've managed to get a simpler workflow. Not saying it'll change much to the overall complexity but it should be way easier to read 😄!

Custom directive instead of built in reactive forms directives

As you've probably noticed by now, we've tried to keep ngx-sub-form as a utility library. We've considered multiple times going all in and create a module that'd contain some directives, could have services etc. But we managed to avoid that (so far).

this.form.attachSubFormComponent(this.host)

As much as I like the idea, I think this would have quite a few issues with the expression has changed error.

I'd be super keen to know what you think. I understand this would be a complete departure from how the lib works today.

Some very inspiring ideas but I'm afraid some of them may not work properly in Angular world. But it might be definitely worth a try with at least a POC. The biggest thing holding me back though is the fact that sub forms could only work well together with sub forms from the lib. Imagine you want to consume from an external lib a sub form (which is implemented simply using ControlValueAccessor), that one wouldn't report up to the parent. But I hear you and understand the frustration around some edges with ngx-sub-form not being able to handle all the use cases for now.

First off sry for spamming this thread

Please don't apologize for that. You're raising really good points and I hope you keep doing that! 😸

ngOnInit on the SubFormComponent gets called prior to ngOnInit on the directive which injects the FormGroup instance into the SubFormComponent

This is no longer an issue with the rewrite I've been working on. I hope to be able to clean up a bit and share some code that you can look at soon, even though it's definitely not finished.

So far, I managed to:

• get it working without inheritance for sub form and sub form remap 🎉
• type safety is pretty good (better in some cases!)
• I've got access to the nested errors
• works well with arrays too (+ errors)
• deprecated formGroupValues as the formGroup.value now have proper types (thanks to you! 🔥)
• deprecated formGroupControls as the formGroup.controls now have proper types (thanks to you! 🔥)

What I couldn't get working well is the emitNullOnDestroy (which is annoying 😓) because of angular/angular#36776 where basically it's not possible to override life cycle hooks on a component instance anymore 😭. I really hope this PR angular/angular#35464 gets merged soon 🙏!

This is a critical blocker as user should NOT have to place his init code in a custom method instead of ngOnInit which would really be the only way around this.

Agree. And I have a nice/safe way of hooking into a component instance to then expose all the life cycle hooks as observables 🔥... But as explained above it doesn't work with Ivy anymore...

Since a SubFormComponent is well defined, we could simply enforce a simple input property e.g. subForm which holds the SubFormGroup instance

I like that because I think we could get better type safety out of it!

its now @input() subForm : SubFormGroup

How would that work?

Couple of lines of code are better than thousand words (basics are already there and have it up and running in our project): https://github.com/ntziolis/ngx-sub-form/tree/sub-form-controls-simpler

Wow never thought of that! That might actually be pretty nice yes but I'm unsure how you'd use that. Do you have any example consuming that new API?

[ntziolis]

First off, hats off to being open to input like that. I call this out as this has not always been my experience when providing feedback especially if it goes to the heart of a project.

I'd love to share the actual project I used while testing this, but as I was crazy enough to put it in one of our actual projects (to see where things blow up beyond simple usage). Hence, below is an outline of the key pieces.

If the parent is a root-sub-form (assuming the old structure for now):

// on the parent component
getFormControls(){
  return {
    // the key here is to use the "special" SubFormGroup class 
    // which understands that there is a sub-form below to be handled
    // no value can be supplied here, but validators / form opts can be provided
    mySubForm: new SubFormGroup() can be provided
  }  
}

// and in the parent template
<my-sub-form [subForm]="formGroupControls.mySubForm">

That's really is all you have todo to hook things up.

The ngOnInit method on the ngxsub-form base class is where the magic happens. It "injects" the sub-form component instance into the SubFormGroup to handle transformations

You raise a good point around interoperability with non ngx-sub-form parents. This would also have been a full stop blocker for us if not possible.

Here is what this would look like:

// on the parent component
this.form = fb.group({
   // syntax is really the same as for non sub forms
   mySubForm: new SubFormGroup()
})

// and in the parent template
<my-sub-form [subForm]="form.get("mySubForm")>

So yes, there is a dependency on usage of SubFormGroup:

• but my-sub-form has to be imported from a lib as well, in that case the lib simply also exports SubFormGroup
• also creating some sort of FormGroup on the parent when feels natural to angular
• in fact angular recommendation in regards to manual sub form handling is to pass the FormGroup instance down which is exactly what we are doing
• the only difference would be to use SubFormGroup instead op FormGroup
  • inside the ngOnInitim checking already if somebody used the correct type and provide a meaningful error message if that's not the case (runtime only though right now)

Considering the above this feels like a good integration story that external devs can easily understand even if they have never seen ngx-sub-form.

However should this would not be acceptable, it would also be possible to create a generic class that wraps a sub-form into a ControlValueAccessor enabled component . It would be up to the dev of the lib to decide how he wants to expose the sub forms then.

Further usage:

• We should also be able to enable usage of subFormName
• I'm not sure sure yet if it would require a directive or not
• Right now I'm tending to no directive is necessary.

[ntziolis]

I'm going to the rest of you comment and reply back later. Just wanted to make sure you have usage samples first.

[ntziolis]

Sry for the long silence. Work got in the way :) over the weekend I had time to migrate the demo project in the repo to use the outlined approach. Need to put some finishing touches on it and will report back here.

[ntziolis]

While building the below I kept in mind that we might wanne move away from inheritance. I do have some ideas how to make the approach below work without inheritance as well. I just based it on what was already available.

Status updates

• demo app is "working" and can be found here
• the code is raw and not yet optimized or even cleaned up, I was trying different path ways back and forth. so please be gentle :)
• the good news is that I was able to work through the main issues that I had in the back of my mind which might be blockers for this path
• there is still an expression changed issue related to the various helper properties formGroupValues / formGroupControls etc.
  • this would be likely solved by moving to what you suggested (using the formGruop directly and dropping some of those properties)
• due to this form status not being updated the droids cannot be upserted
• to reset the app I have added a routerLink to the header, so any time its stuck just click the header
• the lib cannot be compiled yet as the compiler for libs is stricter than ng serve, but this should only be a question of tricking the compile to just shut up :)
• im confident this can be solved, my focus was simply to get all the wiring done incl transforming values

The "good" thing no matter where this leads is I learned A LOT about reactive forms, not sure I wanted to know all that but I'm sure it will come in handy moving forward :)

[ntziolis]

So there seems to be some movement on forms https://www.twitch.tv/videos/620888602, trying to setup a call / twitch session

[ntziolis]

Very interesting read. I knew of mixings, but I didn't know about component features. Together with Feature interfaces its a great way to enforce structure of a component without inheritance: https://indepth.dev/component-features-with-angular-ivy/

[maxime1992]

Hi @ntziolis 👋

Brandon offered Zak and I on Twitter to talk about ngx-sub-form in his live stream 😄 We said we were interested but at least once this issue is closed. I think we can indeed push the architecture further and solve more issues but getting rid of inheritance won't be too much troubles for existing code base and will already solve quite a few annoying ones.

About the component features, we discussed it with @zakhenry and it could be a great match for us. Unfortunately this API is not public and I read comments on Twitter saying that the Angular team was not sure to open this up for public use as it gives them flexibility internally but exposing it would limit the changes they could make to it... So pretty unsafe to use as of today IMO.

I haven't taken a look to your demo yet but will do asap :)

[ntziolis]

Great to know Brandon reached out already, From the session so far I feel they are just barely scraping the surface. I don't think reactive forms in "simple" is bad at all. Sure they could be type safe + make it even easier to create single field sub forms.

But the main issues come up when forms get more complex and you wanne do proper multi field sub-forms. The other key missing piece is transformation which is especially important to us since we don't have control of the downstream systems (backend) in most of our projects.

To give some context we do a lot of "digitization" projects for enterprises. Usually this either means:

• the process was paper or excel based previously and needs to look exactly the same just in the browser afterward
  • This means you usually end up with tons of fields with previously soft defined rules how data is validated
  • We are not recommending the 1:1 approach that but thats where customers usually start as their employees would not need to be retrained
• or there was some sort of digitized version of the process, but developed within systems like sharepoint, salesfore or some other "crm" with non user friendly UI
  • Our task then usually is to collect the same data but in a user friendly manner and save it back to the downstream systems.

Its not uncommon for a single form to have 100+ fields. In addition in all cases the form validation requirements are very complex, here is an actual sample:

• when the value of a form control on form A is set
• and a specific control value on on form B has value x
• and a specific form control on form C is not disabled
• THEN
  • a specific form control on form D is required
  • and pre-filled with value from a control on Form E

This would be a pretty standard use case we have to deal with (sadly im not kidding). Sometimes we have dependencies between 20+ controls on different forms to make a decision on a specific form control.

Given our project types our work is extremely forms heavy. I mean 90%+ of our work is building forms as auth, hosting, backend is handled by downstream systems and is out of our control. We have likely crossed the 2-3k forms mark by now for the last year alone.

Maybe with the above it becomes a bit more clear why I'm desperately trying to figure out if we could make sub-forming work without loosing access to the underlying controls.

I understand that our requirements are on the upper end of complex, but still hope that this could be made in a way that it doesn't add complexity for the simple use cases.

Obviously I'm happy to participate in a call / witch session if you feel it would be helpful. Just lmk.

[hakimio]

Its not uncommon for a single form to have 100+ fields. In addition in all cases the form validation requirements are very complex, here is an actual sample:

• when the value of a form control on form A is set

• and a specific control value on on form B has value x

• and a specific form control on form C is not disabled

• THEN

  • a specific form control on form D is required
  • and pre-filled with value from a control on Form E

You might want to take a look at Akita Forms Manager.

[ntziolis]

@hakimio Thank you for the suggestion (might be super helpful to others who find this thread). We already use it heavily and it cannot be stressed how super helpful it is :)

Sadly though there is no access to the individual form controls down the tree when using it together with ngx-sub-forms. The controls on sub-forms below the root form are hidden by ControlValueAccessor. Why does it matter:

• no way to access pristine / dirty / touched /value changes of individual controls from outside the sub-form
• no way to set validators / deactivated on individual controls from outside the sub-form
• basically no access to any functionality of individual controls from outside the sub-form

The from outside the sub-form really is the key piece here and for better or worst is our default use case.

We could not use ngx-sub-forms, but we are in love with value transformations and the structure it imposes. Switching projects or onboarding new devs is a breeze (vs pain in the ass) since every form has the exact same structure. Hence the willingness to try out different paths to get the best of both worlds.

[andreElrico]

• I think "get rid of inheritance" will be a big improvement. Thats the one thing I really disliked about this project.
• Also I've read that @angular is starting to work on strongly typed forms, so maybe this will make this lib even more lightweight.
• I also read here about Ivy-features, this could def. be something to investigate, However if this means we loose freedom for simplicity again.

That being said, native strongly typed forms and ivy-features will probably not land very soon as framework changes progress slowly.

Just to add some feature request.

• It would be cool if polymorphic forms would be able to realize with ng-templates there is templateGuards to maybe this could be used to add typesavety.
• Also the polymorphic forms api was always quite challenging for me. Maybe there an easier way :/ .

Im did not use ngx-sub-form lately, but I remember the countless times you helped me out @maxime1992 . Dont worry you two (+ @zakhenry ) about being inactive. Looking forward to ngx-subform2.

[ntziolis]

Was sadly to busy until recently. I was able to make major progress and now only have 2 issues left that I'm confident I'll be able to resolve it this "sprint".

Issue resolved:

• Various ExpressionChanged errors (this was a blocker issue and by far the most complex of the remaining)

What was changed:

• It became clear that there is no way to make this work without access to ChangeDetectorRef
  • However simply adding this to the constructor of sub-form-component would have casued all derived sub forms to have to pass this down the chain
  • Instead its now using a directive that gets ChangeDetectorRef injected via DI and then pass it ot the sub-form-group
  • The directive uses subForm as selector
  • Since every sub-form-component already has an input with that name which gets passed the SubFormGroup there is no additional code that the user needs to write to gain access to the ChangeDetectorRef

Remaining issue are:

• Incorrect form validity of droid form after the form was updated via input (old DataInput) into the root form
  • Initial load works without a problem as well as when changing droids
  • Also the form is valid again after any change made
  • This is also not a general issue with the new way DataInputworks since the same issue does not appear on the more complex vehicle form (incl array)
  • Hence I'm confident that this is very likely a minor issue and the main task will be tracking it down
• formGroupErrors needs to be reworked to proactively regenerate when form status changes
  • there is no blocker that i can see at this point, I simply didn't get to it yet as it was lower priority

@maxime1992 @zakhenry You more than are welcome to review the currently pushed state. After I have resolved the last issues I'd love to have a chat with you to walk you through it in detail. You can then decide if this is something you wanne incorporate in to this lib or if I should separated it out in a new lib (my team has already played around with this extensively and it solves all the issues we were running into that I have outlined previously).

[ntziolis]

One down one to go:

• validity issue is solved now
• turns out the root cause was the same as the referenced link in the demo app
  Calling setValidators on a form control when control is nested as a component in form causes ExpressionChangedAfterItHasBeenCheckedError angular/angular#18004 (comment)
• Key is to call hook into AfterContentChecked and execute detectedChanges at least once in the sub form tree. I have already optimized for this to only happen on the root form and all sub forms to only call markForCheck
• the user can inject ChangeDetectorRef in his root form constructor and pass it down to the base class
  • this is fully optional, but if not done the sub forms will all call detectChanges instead of markForCheck
  • meaning the recommendation is to pass it to the root form base class

This means:

• When cd was injected into root form base class there is now no need for nesting in the form, via *ngIf="true"
• If not injected user still needs to use the wrapping workaround

I have also started to clean up a bit.

Will focus on getting formGroupErrors to work as it did before:

• right now there it does work but does not contain all the errors of decedent sub-forms yet.
• In fact I'm rethinking if this even makes sense and not just output the errors on the current sub form level as this is only meant as a shorthand for that level.
• there is no typing for the level below the current sub form level anyway
• errors on any other part of the sub form tree can be accessed via the standard formGroup.getError
• it only would make sense to provide errors from descendent sub forms in formGroupErrors is when it would be fully typed down the entire sub form tree

Le me know what you think on the error front.

[maxime1992]

Hi @andreElrico 👋

I think "get rid of inheritance" will be a big improvement. Thats the one thing I really disliked about this project

We're conscious this is not ideal but as form should have tiny scopes it wasn't acutally so bad IMO but still trying to come up with something better :)!

Also I've read that @angular is starting to work on strongly typed forms, so maybe this will make this lib even more lightweight

Yes, I'm following some issues too and I've seen some comment from Kara :). Although types are not the biggest part of this tiny code base, I'd be glad to remove them if there's a good native support 🙌

That being said, native strongly typed forms and ivy-features will probably not land very soon as framework changes progress slowly.

Yes I doubt we're going to see that landing in the framework any time soon but 🤞

It would be cool if polymorphic forms would be able to realize with ng-templates there is templateGuards to maybe this could be used to add typesavety.

Not sure to understand what you mean here?
Are you talking about something like type safe switch case in TypeScript? If so I've created an issue in July 2018 for that: angular/angular#24978 Feel free to give it a 👍

Also the polymorphic forms api was always quite challenging for me. Maybe there an easier way :/ .

Could you be more precise?

Im did not use ngx-sub-form lately, but I remember the countless times you helped me out @maxime1992 . Dont worry you two (+ @zakhenry ) about being inactive. Looking forward to ngx-subform2.

😸 🙏

---

Hey @ntziolis 👋,

thanks for putting so much effort into all this 🔥

Was sadly to busy until recently. I was able to make major progress and now only have 2 issues left that I'm confident I'll be able to resolve it this "sprint"

Us too. We've had 1 or 2 sprint in which we managed to save a lot of bandwidth for ngx sub form rewrite but then had to move on before I got done with it.

About the rewrite, I got pretty far and most of the e2e test are working but to get the errors correctly working without having an expression has changed error is a nightmare (as you seem to be aware ha). If you want to have a look into the progress and the rewrite you can here: https://github.com/maxime1992/ngx-sub-form/tree/feat/rewrite 👍

Various ExpressionChanged errors (this was a blocker issue and by far the most complex of the remaining)

I feel your pain. This is one of the key point of ngx sub form. Deal with those nested structures that can easily end up with expression has changed errors so that people don't have to deal with them in their code base 🔥 😄 💪!

It became clear that there is no way to make this work without access to ChangeDetectorRef

The library is using ng9, do you think the new (but private 😞) function markDirty may be enough? If so I think we could use that and if it ever gets remove then ask people to provide the ChangeDetectorRef... But in the meantime it'll still mean that we don't need to provide anything and add any boilerplate :)

After I have resolved the last issues I'd love to have a chat with you to walk you through it in detail

Yeah I'd be happy to hear more about what you built as I've not been able to find some free time to dig into it yet! Don't know when we can do that but I've created a Discord server for ngx-sub-form so that we can easily jump into a visio conf when we feel like it :) Feel free to join here and we'll try to figure out a good time to chat: https://discord.gg/wNFg7ab

I have also started to clean up a bit. Will focus on getting formGroupErrors done, hopefully by eod tmr

Good luck!

[maxime1992]

Hello 🌞

@zakhenry and I found some time during the weekend to continue a bit on the rewrite and I believe we reached a state where we can raise a public PR and let anyone have a look to the implementation and the new pubilc API 🎉.

The PR is here: #176

We've duplicated our demo app to have both the old one and the new one (same but using the new API) and our E2E tests have been updated too to run on both apps:

It looks pretty promising but maybe some cases are not handled by our E2E tests so we'll need a solid help for manual testing and everyone can help by giving a go at the new version 🙌.

Few things are still breaking changes at the moment but we'll do our best to make sure we have the old and the new version coexisting for a little bit so that people can have a smooth upgrade instead of a painful, "big bang" one.

As I said, still a WIP but we'd love to have some feedback onto it!

This wouldn't solve all the issues... But would still solve the following ones:
(appart from the oneOf which is unrelated here)

Currently the only way to give it a go is by cloning the repo, checking out the rewrite branch and building it. We understand it is pretty painful and we'll try to have a pre-release setup on that branch to make this easier for everyone who wants to help testing it.

⚠️ Please don't rewrite all your forms yet ⚠️
This is still a WIP and the public API may change if we discover major issues.

The best way to start would be to have a look on the PR #176 and share your thoughts then look into the PR within our demo code to see how to use the new API. You could then give it a go on your side on some forms 👍

Let us know what think, please don't be shy 😸 !

[evgeniyefimov]

Any ETA for these changes?

[maxime1992]

ETA:

• PR [WIP] feat: rewrite of ngx-sub-form without inheritance #176 is pending as we haven't had the time to get back to it for a while ⏳
• I believe most of the work for the refactor has been done and from the few tests we've done on our demo app it looks promising 👍
• A demo app may not be enough to discover all the edge case unfortunately ⚠️ We need feedback from the community to make sure we're not breaking anything. At some point I'll give this a go myself onto our own app at work but as I don't have much time right now any confirmation from the community that things are working ok with the new architecture would be much appreciated 🙏
• I do understand that the current state is not ideal for people who wants to give this a go as we don't have any pre release setup nor documentation to explain how to build it and use it locally on your project. I think this should be done asap so that hopefully we can gather feedback from the community. Assuming we manage to publish a pre release version on npm, who would be up for some beta testing? Please put a 🚀 on this comment if you're willing to help so I can ping you directly later on. We'll do our best to have a release containing both the current code AND the new one so that the update can be done incrementally
• E2E tests are fine ✔️ 🔥
• We've lost I believe all the unit tests 😬 Not sure how much we'll be able to reuse but probably not much so this will take time...
• Documentation hasn't been updated yet (and shouldn't) until we get some feedback to make sure the API is fine

I'm not 💯% sure of that yet but I think that as soon as we've got some feedback and if the API looks good, we may merge into master and just provide 2 different paths to import from. The regular one and maybe a new one like ngx-sub-form/new-api or something similar. This way we could incrementally improve this instead of having a long lived branch which may become an issue.

[maxime1992]

Update: I've spent some time this week end to:

• split the "old" API and the new one in 2 different paths: ngx-sub-form (no breaking change 🙌) and ngx-sub-form/new (may be renamed before merging)
• try to publish a pre release for this branch

The split is done ✔️ but unfortunately I hit an error when I try to make a pre release.
I've open an issue here semantic-release/github#285 and it may be related to the name of the branch so hopefully I can solve that soon 👍

[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 📦🚀

[maxime1992]

Wouhouuu it worked! 🎉

If anyone wants to try this out, ngx-sub-form@5.2.0-feat-rewrite.1 is available!

Haven't tried it myself yet as it's a bit late but... 😬 🤞

I'm going to close #176 in favor of #188 which is the same of except that I renamed the branch to go around the publishing error with the not supported format for the branch feat/rewrite which I renamed in the new branch to feat-rewrite.

[maxime1992]

Hello 👋!

📢 We've got some important updates to share! 📢 😄

Some work has been done over the weekend on the refactor to make sure everything is working fine.
To do that, what's better than a demo app? A real world app!

So we've upgraded our ngx-sub-form version in our monorepo to v5.2.0-feat-rewrite.6❗
As we can use ngx-sub-form and ngx-sub-form/new this didn't bring any breaking change 🔥.

As the update can be incremental, we haven't committed yet to transform 100% of our forms. BUT. We did update a part of our app that contains a lot of root and sub forms. The diff before/after when searching for from 'ngx-sub-form' is 131/79 😸!

The idea behind giving this an early try is to:

• make sure nothing is broken after upgrading from v5.1.2 to v5.2.0-feat-rewrite.* and that there are no breaking changes
• make sure we've got access to everything we need to migrate under the import ngx-sub-form/new
• make sure nothing was fundamentaly broken in the new module
• make sure nothing was forgotten and that all the features previously available are in the new one
• thinking a bit how to draft a potential release soon and explain clearly how to do the upgrade

Guess what? We're glad we did it because we caught quite a lot of issues, missing features and we learnt a lot about the process of upgrading an existing code base! Our internal E2E tests on our app got us covered and caught a lot of regressions which we were able to identify and fix one by one (note the bump from v5.2.0-feat-rewrite.1 to v5.2.0-feat-rewrite.6 in 2 days 😂).

But the good news is: Our internal codebase is now running on v5.2.0-feat-rewrite.6 and should go in prod pretty soon 👀.

What's coming next?

• Mark everything in the previous code base as deprecated
• Some clean up required on the new code
• 0 unit/integration tests right now for the new code :s
• Documentation to update
• Prep a release note explaining how to upgrade an existing code base incrementally

As things are looking pretty good but not ready to be released yet, I'll try to draft up a guide that'll reuse later on in the release note on how to migrate so that if anyone from the community feels brave enough to give a go to the new version, we'd love to hear some feedback onto it 😸!

I don't think the API on the new part is going to change a lot but keep in mind that those are just pre releases and we may modify slightly the API so if you're willing to maybe give it a go on a small part of your code base but if you're using ngx-sub-form a lot don't update everything just yet.

We'll be in touch soon 👋

[maxime1992]

🔊 NEED INFO 🔊

Who's using handleEmissionRate in his code base?

We do expose that method so that automatic root forms can be debounced for example:

protected handleEmissionRate(): (obs$: Observable<FormInterface>) => Observable<FormInterface> {
  return NGX_SUB_FORM_HANDLE_VALUE_CHANGES_RATE_STRATEGIES.debounce(300);
}

But this method is exposing the final observable (that is going to be sent to the parent through the Output) and this is something odd. Surely we shouldn't be exposing the form value in that hook. I needed it pretty quickly so in the new code base I did reimplement the same thing (as an object of the configuration now):

public form = createForm<ToolHolderFilter>(this, {
  formType: FormType.ROOT,
  // ...
  handleEmissionRate: NGX_SUB_FORM_HANDLE_VALUE_CHANGES_RATE_STRATEGIES.debounce(300),
});

I'm wondering: Who's using this hook with something else than a debounce?

Could we simply turn that into

public form = createForm<ToolHolderFilter>(this, {
  formType: FormType.ROOT,
  // ...
  emissionRateDebounce: 300
});

❓

We've never used it for anything else different than a debounce on our side but wondering if we're missing something where it could come in handy to have a different operator?

[agallardol]

Hello, thank you very much for the library, in some of our projects in Cotalker we implemented a lot of boilerplate similar to that of ngx-sub-forms to be able to build "subforms" in the cleanest way possible.

Some days ago we started using ngx-sub-forms and it was a good experience until we came across issue #93 , then we noticed that it had fixed it in version 5.2.0-feat-rewrite.1 and we thought OK let's install the version 5.2.0-feat-rewrite.8 which should be the most appropriate before breaking changes. We did all the necessary refactors but when trying to use 5.2.0-feat-rewrite.8 the angular builder threw us the following error:

ERROR in The target entry-point "ngx-sub-form" has missing dependencies:
[ng] - ngx-observable-lifecycle
[ng] - tsdef

So we thought, OK maybe it is more appropriate to use the latest version of rewrite (6.0.0-feat-rewrite.5)?

For this reason, before continuing to implement code, I would like to ask you what is your view on this. Say, from what version should we start using ngx-sub-form? We wouldn't want to have to deal with the aforementioned 5.1.2 bugs or have to refactor our code when you release 6.X.X.

The library's new approach in "feat-rewrite" branch that relies on composition rather than inheritance looks great. Thank you very much for the tremendous work you do.

[maxime1992]

Hi @agallardol thanks for the kind words and happy to see you starting using ngx-sub-form!

There's a comment here: #188 (review) where indeed this was reported and we haven't fixed this yet. In the meantime you can simply add those packages directly as a quick fix with npm or yarn and you'll be fine.

About the new published versions 5.1.2-feat-rewrite.* and 6.0.0-feat-rewrite.* here's some clarification:

• We initially started this rewrite 5 months ago
• ngx-sub-form never got updated to angular 10 and we were still on 9 (not a big issue really but just a fact)
• We started the rewrite as a non breaking change by keeping both the old and the new codebase
• We encountered an issue where we couldn't patch a component directly ourselves because of a regression brought in Ivy and @zakhenry built a wrapper to manage that case as simply as possible (through a decorator)
• This issue got fixed in the meantime and therefore we decided to take advantage of this as it lets us clean up a huge part of the code to go around that
• We therefore upgraded the rewrite branch to Angular 10 (this is the breaking change)

Now, if you feel adventurous and you're happy to help us test out the pre release I'd definitely recommend to be on latest 6.0.0-feat-rewrite.*. This is the most recent and this is what we're going to build on from now on (till of course we finally merge the rewrite!).

We've migrated one of the most important bits of our internal app and containing a lot of (ngx-sub-form) forms a month ago or so. We've discovered issues that have been fixed and we're fairly happy with it so far. But once again, this is a pre release so if you see anything going wrong please report here :).

On the documentation and migration topic, we've started to work on the documentation to help people migrate to the new version asap. The doc is not ready yet even as a draft but we're working onto it. In the meantime, your best bet is to simply look at our examples here: https://github.com/cloudnc/ngx-sub-form/tree/feat-rewrite/src/app/main-rewrite We've copied the demo app (main folder --> main-rewrite) and you'll be able to get a sense of how things should look like by looking at all the forms in that new folder. None of the existing concepts have change (root forms, sub forms, etc). It's just a matter of writting things slightly differently to get rid of inheritance but we tried to keep the same names etc. The biggest change is that to create a form we do that through a function and instead of having different methods in your class to define the form you now pass it as a param of this function.

If you're missing some info from those examples, feel free to ping here too while the documentation isn't ready and don't be afraid to share some feedback on the new API 😄!

The library's new approach in "feat-rewrite" branch that relies on composition rather than inheritance looks great. Thank you very much for the tremendous work you do.

Much appreciated by @zakhenry and myself! Thanks 🙏

[Waterstraal]

Hey, I was feeling adventurous and decided to try the latest published version: 6.0.0-feat-rewrite.15. I have to say, I really like the new api; much cleaner imho!

Hower, I do have 1 question: Like @agallardol also mentioned, I was also taken by surprise that I had to install 2 additonal packages, that I did not need to install before: ngx-observable-lifecycle & tsdef. Will these 2 dependencies still be needed in the final version? I feel like this library would be much better as a stand-alone lib without external dependencies (apart from Angular of course).

Cheers!

[maxime1992]

I was feeling adventurous and decided to try the latest published version: 6.0.0-feat-rewrite.15

😸 👏!

I have to say, I really like the new api; much cleaner imho!

Awesome, thanks for letting us know!

I had to install 2 additonal packages, that I did not need to install before: ngx-observable-lifecycle & tsdef. Will these 2 dependencies still be needed in the final version? I feel like this library would be much better as a stand-alone lib without external dependencies (apart from Angular of course).

Oh yep we've probably just forgotten to add them as peer dependencies so they auto install 🤐 Will do.

• ngx-observable-lifecycle is a library that we've pretty much created for ngx-sub-form :) If you look at the implementation behind the scenes of the new API, we need to hook into the current component's lifecycles and as it was something that needed to be done in a very safe way to not mess things up, it's been done and tested in its own library. It's also one that we built with a reactive API in mind as we love observables and behind the scenes the new ngx-sub-form API works this way as well. You can have a look at it here: https://github.com/cloudnc/ngx-observable-lifecycle/ It's really not a huge library and to answer your question: Yes we need it, yes it'll be in the final version. We'll just make sure it's auto installed and that you don't have to do it yourself
• tsdef is a very very tiny library which provides some helpful Typescript types. As we've been trying to have as much type safety as we could, we rely a lot on some powerful types. No need to re-invent the wheel when someone managed to make a good library for it AND especially find some good naming for all those types 😬! That said, this one will not end up in your final prod (or even dev) bundle as they're just types. Once Typescript has done his thing and generated the library there will be literally nothing left of tsdef in your bundle. Feel free to have a look at it here: https://github.com/joonhocho/tsdef

If you start using the new API everywhere and migrate your existing forms to it let us know more along the way if it's smooth, if you encounter any issue etc. May help us write some better migration notes :)

[Waterstraal]

Hi @maxime1992,

Is this project still under active development?

[maxime1992]

Hello @Waterstraal. I can't say it's active I'm afraid. The refactor has been going for over a year without being merged to master.

That said. We do use ngx-sub-form regularly in our codebase at work and maintain the library. But mostly based on our own planning when we need things.

It means that the beta branch and release are still quite active, see #188

What's left before we can merge the beta branch into master is:

• Update documentation
• Write tests (we've got e2e, unit would be nice even though we're at a point where I'd be willing to ship first and add tests along the way...)

Assuming you're already familiar with ngx-sub-form API, you probably don't need any documentation to start using the beta version (available on npm). You can just take a look here: https://github.com/cloudnc/ngx-sub-form/tree/feat-rewrite/src/app and compare the 2 folders main and main-rewrite which are the same demo app but main written with the old api and main-rewrite being written with the new api.

Hopefully at some point we find some motivation to update at least the documentation.
Just lots of things have been going on in the last year for both Zak and I and we haven't had much time to dedicate to OSS.

I think I'll have a go soon at upgrading for ng 13 though so that people using ngx-sub-form aren't stuck and can at least use the beta branch with their project. Note: Even if it's called beta it's stable now and we don't plan on introducing any further breaking change I think.

[maxime1992]

@Waterstraal I've raised a MR to add support for Angular 13 and avoid the ngcc step for ngx-sub-form :)

Note: This MR targets the feat-rewrite branch (the one published on the beta channel).

[maxime1992]

Alright, I've tried to find some time... and strength to rewrite the README to explain what's ngx-sub-form, how to migrate to the new api if you've been using the old one and also explain all the different principles for the lib.

Meaning, we should now be close to a state where we'll only be missing some unit tests. I think we should be good to release on master because the old API is still here and nothing will be broken by default but people should be able to start using and/or migrating to the new one.

For anyone interested, please have a look here: #231

Probably not perfect but I hope it'll be at least a good start. I've been too head down into this to actually realize whether its any good or not and no one new to the project would understand what I wrote. Don't be shy and let a comment directly on the MR to let me know what you think, or even raise a MR targeting mine if you think you can improve it.

[Waterstraal]

👍 sounds good! I'll try to have a look next week 👏

[maxime1992]

Good evening everyone 👋

Looks like after a year and a half... We're finally merging this 🎉

Feel free to give us some feedback how the new API is working out for you. Hope you enjoy the bug fixes and the switch from inheritance to a function createForm (see updated README!) 😸.

Thanks to everyone who participated in this thread to give feedback and ideas 👏 and a big thanks for all your patience as well 🙏!

[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 📦🚀