Skip to content

feat: Update to Angular 14 #44

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Dec 9, 2022

Conversation

jafin
Copy link
Contributor

@jafin jafin commented Dec 9, 2022

Update library to build on Angular 14.

  • doc: minor doco change.
  • task: build pipeline to Node16 (from 14)
  • fix (Compiler): fix error NG3001: Unsupported private class ValidationMessageComponent, ValidationMessagesComponent FormDirective by adding to public_apis
  • fix (ng-packagr): Configuring ng-packagr in "package.json" is deprecated. Use "ng-package.json" instead.
  • Resolves Upgrade to Angular v14 #37

jafin and others added 3 commits December 9, 2022 13:59
- Expose additional components due to Typescript requirement.
- Minor doco updates.
- Build pipeline bump to Node16 (from 14)
Copy link
Owner

@davidwalschots davidwalschots left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks again for your work @jafin. It's much appreciated.

At work I'm still using an Angular version < 14, so I've not yet familiarised myself with the details of typed forms. At the moment your PR modifies the code such that it works with untyped forms.

I don't know if this would already work with typed forms? If so, then we might want to consider adding some tests for it. If not, and the changes necessary to make it work are minor, then we might want to consider supporting it within this PR. If typed forms requires more changes, then we'll just release this and consider it a separate feature. In that case we might want to put a warning in the documentation that only untyped forms are supported at the moment.


export const getFormControlFromContainer = (name: string, controlContainer: ControlContainer | undefined): FormControl => {
export const getFormControlFromContainer = (name: string, controlContainer: ControlContainer | undefined): UntypedFormControl => {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose we want to support typed forms as well? I'm not sure if this change handles both typed and untyped forms. It might be a good idea to add some typed form tests and check.

The Angular documentation lists an UntypedFormControl as an alias for FormControl<any>, but I'm not sure what the runtime behaviour of it all would be.

Copy link
Contributor Author

@jafin jafin Dec 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have the knowledge on this one at the moment, I honestly just did the bare minimum for the upgrade and my understanding was that the untyped maintained compatibility without any changes. Additional tests would be good yes.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In src\app\app.component.ts I changed UntypedFormBuilder to FormBuilder. This still works fine.

I suspect this is what happens:

  • type is a TypeScript construct which needs to be compiled away. Thus type UntypedFormControl = FormControl<any>; becomes FormControl<any>.
  • Generics are also a TypeScript construct which needs to be compiled away. Thus FormControl<any> becomes FormControl.
  • A typed form control would have say type FormControl<string | null>, which then would also become FormControl.

Thus a line like if (!(control instanceof UntypedFormControl)) { actually compiles to if (!(control instanceof FormControl)) {.

I'll release a new version somewhere later today or tomorrow.

}];

combinations.forEach(combination => {
combination.controls.forEach(control => {
const nativeResult = combination.nativeValidatorFn(control);
let libraryResult = combination.libraryValidatorFn(control);

libraryResult = libraryResult;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes me wonder if I was slightly drunk at the time 😀.

@davidwalschots davidwalschots merged commit 14ae930 into davidwalschots:master Dec 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade to Angular v14
2 participants