search

gabrielguerrero / ngrx-traits

NGRX Traits is a library to help you compose and reuse state logic in your angular app. There is two versions, @ngrx-traits/signals supports ngrx-signals, and @ngrx-traits/{core, common} supports ngrx.
MIT License
66 stars 3 forks source link

Migration to Angular/NX v15 #39

Closed DelBiss closed 6 months ago

DelBiss commented 7 months ago

Migration executed by Nx with the command 'nx migrate 15.9.3', the latest non-major version of v15

Those main dependancies have been updated by the automatic migration:

The migration caused some TypeScript error, but also problems with other dependancies that needed to be updated:

About the Typescript Issue following migration from 4.7.4 to 4.9.5

Most of the error was due to lack of constrain on certain type. There was also some weird one in async-action.trait.ts and comment have been left to explain them.

The most noteworthy one is related to KeyedConfig and AllTraitConfigs (because of the TODO comment on line 75. The change that I've made is the following:

  - KC = KeyedConfig<KEY, C>
  + KC extends AllTraitConfigs = KeyedConfig<KEY, C>

What I would like to note is that this following change look like to also work:

  - KC = KeyedConfig<KEY, C>
  + KC extends KeyedConfig<KEY, C>= KeyedConfig<KEY, C>

I've chose to go with AllTraitConfigs since it's the least constrain needed to make it work.

Deprication warning on RxJS

There was 2 deprication warning in trait-effect.spec.

The first one concerning mapTo(val). It have been replace by map(()=>val) has suggested.

The second one's concerning the usage of toPromise() on Observable. I've tried to implement the suggested change, but it doesn't work. Maybe it's because I didn't do it properly or their's a bug in the test or effects. Since the test work without the change and a deprecation warning, nothing have been change for the moment other than commenting.

Final Note

DelBiss commented 7 months ago

Also, note that I'm currently working on migration to v16

gabrielguerrero commented 6 months ago

@DelBiss Thanks a lot for this will check on it and try to merge it soon

gabrielguerrero commented 6 months ago

@DelBiss the PR looks good just some small comments, I will like to merge this let me know if you have time to do the changes I requested Or I can do them, I saw some where you were also trying to migrate to 16 let me know if you made any progress on that, otherwise I can do it, I want to migrate to 17 there is a new package Im building that have traits that work with ngrx-signals that needs the version 17

gabrielguerrero commented 6 months ago

@DelBiss Im gonna be merging this into the beta branch instead where I will do a few tweaks and merge your other branch there as well so I can do a release, I already tried migrating locally to 17 and was easy so I suspect 16 is the same. Thanks again for your help.

github-actions[bot] commented 6 months ago

:tada: This PR is included in version 15.0.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket: