simontonsoftware / s-libs

A collection of libraries for any of javascript, rxjs, or angular.
MIT License
43 stars 5 forks source link

feat(ng-core): Select WrappedAbstractControlSuperclass #52

Closed A77AY closed 2 years ago

A77AY commented 3 years ago
ersimont commented 3 years ago

Thank you for filing the issue, and making the pull request!

Can you give an example of a motivating use case for this change? Something to help me understand the purpose, and that could be used in docs/tests?

A77AY commented 3 years ago

@ersimont I would like to use forms like this:

@Component({
  selector: 'app-full-name-input',
  template: `<div [form]="formControl"><input controlName="name" /><input controlName="lastName" /></div>`,
  providers: [provideValueAccessor(FullNameFormComponent)],
})
class FullNameFormComponent extends WrappedAbstractControlSuperclass<{name: string; lastName: string}> {
  formControl = new FormGroup({name: '', lastName: ''});

  constructor(injector: Injector) {
    super(injector);
  }
}

or for example price + currency or something else

ersimont commented 3 years ago

OK! I started playing with this example and I see the appeal. This allows you to use a FormControl, FormGroup, or FormArray and still get the benefits of things like automatically-updated disabled state. I'll try to look more into it soon! I haven't tried submitting a PR to a PR before (because I'll want to e.g. move some tests around). We'll see what I can do :)

ersimont commented 3 years ago

OK I finally got around to paying this more attention! I'm thinking I'll just rename the existing class without keeping a subclass with the old name, like you did here. This means I will need to release it with version 13.

Let me know what you think about what I landed on: https://github.com/simontonsoftware/s-libs/commit/fb7cc7eb9fd052c49bf9132c0f8e40d45f7092b8

A77AY commented 3 years ago

OK I finally got around to paying this more attention! I'm thinking I'll just rename the existing class without keeping a subclass with the old name, like you did here. This means I will need to release it with version 13.

Let me know what you think about what I landed on: fb7cc7e

Looks good (especially tests)

I faced with problem of specifying ngOnInit in inherited components. If you forgot to call parent ngOnInit, you may not understand this immediately. (I still have not decided for myself whether the library should solve this problem or not).

My decision ```ts /** * https://github.com/microsoft/TypeScript/issues/21388 */ export const REQUIRED_SUPER = 'RequiredSuper' as const; /** * If you're here, you probably need to return the parent * class's implementation, e.g. return super.ngOnInit(); */ export type RequiredSuper = typeof REQUIRED_SUPER; class WrappedControlSuperclass { ngOnInit(): RequiredSuper { // ...WrappedControlSuperclass ngOnInit return REQUIRED_SUPER; } } ``` Then the inheritance looks like this (if we do not return RequiredSuper we will get an error): ```ts export class SomeControlSuperclass { ngOnInit(): RequiredSuper { // ...kind of logic return super.ngOnInit(); } } ``` It is also possible to add `beforeOnInit()` and `afterOnInit()` methods (then we can to call one of these methods instead of ngOnInit)
ersimont commented 2 years ago

It looks like I may need to make that simple WrappedFormControlSuperclass after all. Though all my tests passed, I just tried using my version in a real app compiled with Ivy & AOT and it wouldn't compile 😞 😠

ersimont commented 2 years ago

Closing this PR because its features have now been rolled into the version 13 branch.

Thank you again for the contribution!!