Skip to content
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

Upgrade to Angular 15 #282

Closed
ophirKatz opened this issue Nov 22, 2022 · 12 comments · Fixed by #288
Closed

Upgrade to Angular 15 #282

ophirKatz opened this issue Nov 22, 2022 · 12 comments · Fixed by #288

Comments

@ophirKatz
Copy link

No description provided.

@ophirKatz ophirKatz changed the title Upgrade to Angular 14/Angular 15 Upgrade to Angular 15 Nov 22, 2022
@188599
Copy link

188599 commented Dec 6, 2022

Support for Angular 15 would be great.

I added this lib to my Angular 15 project but I had to manually change several typings for it to compile. Weird, considering that typed forms were already part of Angular 14, had to add extends { [key: string]: any; } constraint to all generics, otherwise it would break the build.

@maxime1992
Copy link
Contributor

Hello. I've cut a first release (7.0.0) which removes the deprecated API.

I then tried to give a quick go at the upgrade but it's a bit more painful than I thought and gives me errors around our wrapper types (that were written before Angular forms were typed). I don't think it'll be a quick fix and it needs a bit of a dive to understand whether we still need our wrapper types, if we should only use the ones from Angular, etc etc.

I've raised a draft PR that anyone can take a look at. If you've got ideas/opinions on the most appropriate way to deal with the new typed forms please let me know.

I'm afraid I had to stop here for potentially a while. I wanted to give it a quick go but it needs more work. If anyone is willing to take a look at my PR and eventually give it a go to continue it, please checkout from my branch and raise a PR on your side either targetting my branch or master directly. Thanks!

@maxime1992 maxime1992 added the Help wanted Help wanted from the community label Dec 7, 2022
@188599
Copy link

188599 commented Dec 7, 2022

From what I have seem, most of the things this lib has already covers the new things the official typed forms Angular offers, so I don't really see a need to rush for a change into the new typings.

I'd suggest to keep it simple at first and just change the constraints necessary for the current types from the lib to work well with the new constraints in the new Angular types, and gradually move towards changing things in the backend to the new APIs.

Edit: Looking a bit further into it, the only thing that's broken in the current type seems to be that the type of value in the FormGroup.setValue needs to be of type object.

Changing the two interfaces in ngx-sub-form-utils.ts to following seems to fix the build.

...
export interface TypedFormGroup<TValue> extends UntypedFormGroup {
    value: TValue;
    valueChanges: Observable<TValue>;
    controls: ControlsType<TValue>;
    setValue(value: TValue & {}, options?: Parameters<UntypedFormGroup['setValue']>[1]): void;
    patchValue(value: Partial<TValue>, options?: Parameters<UntypedFormGroup['patchValue']>[1]): void;
    getRawValue(): TValue;
}
...
export interface TypedFormControl<TValue> extends UntypedFormGroup {
    value: TValue;
    valueChanges: Observable<TValue>;
    setValue(value: TValue & {}, options?: Parameters<UntypedFormControl['setValue']>[1]): void;
    patchValue(value: Partial<TValue>, options?: Parameters<UntypedFormControl['patchValue']>[1]): void;
}
...

maxime1992 added a commit that referenced this issue Dec 9, 2022
BREAKING CHANGE: Angular 15 required

This closes #282
@github-actions
Copy link

github-actions bot commented Dec 9, 2022

🎉 This issue has been resolved in version 8.0.0-feat-upgrade-angular-15.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@maxime1992
Copy link
Contributor

Hey @188599 thanks for the snippets. I had a quick look, it wasn't enough to get it to compile fully but I pushed a little bit more and managed to get to a good state I believe. If you could make a review to my PR that'd be much appreciated.

Also, with the new changes on angular material the view got badly broken and the e2e tests needed a bit of a refresh 💅.

As for Cypress, I've faced some difficulties to make it run on CI and had to change how the tests are run a little bit but it's all done, we should be good to merge soon 🤞

For anyone willing to give a go to the beta version with support for angular 15: Use the npm version ngx-sub-form@8.0.0-feat-upgrade-angular-15.1 for now and you should be good to go. Please give some feedback if you want to help get this merged asap :)

(as you can see, the bot above is half correct. It's "resolved" BUT not in the main branch).

@188599
Copy link

188599 commented Dec 9, 2022

Hey @188599 thanks for the snippets. I had a quick look, it wasn't enough to get it to compile fully but I pushed a little bit more and managed to get to a good state I believe. If you could make a review to my PR that'd be much appreciated.
...

I'd be glad to take a look at. And thanks for the quick update!

@188599
Copy link

188599 commented Dec 9, 2022

Perfect! Only had to update rxjs to the latest minor version. 👏👏

@maxime1992
Copy link
Contributor

Checked on our private monorepo at work and all the unit/e2e tests are passing, no blocker on my side. Unless someone give a shout not to merge this I'll release soon :)

@ophirKatz
Copy link
Author

Thank you for the effort. I'll test it today for our repo (holding off still on migrating to angular 15 until more dependencies release upgrades, but I'll still check all things related to sub_form). I'll let you know if all is good or not

@ophirKatz
Copy link
Author

I tested it on our code base. Seems like it works well. So just to be sure, there are no changes to the API, right?

@maxime1992
Copy link
Contributor

There shouldn't be any 👍 I just wanted to double check that the changes on the types wouldn't break anything but they shouldn't really. The breaking change is just for major bump of angular version

@github-actions
Copy link

🎉 This issue has been resolved in version 8.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants