How to deal with breaking change

[GregOnNet]

In NgRx Ducks 9 a Breaking Change is about to be introduced in order to align the API.

Methods annotated with Action are not dispatched anymore.
They will just be plain action creators.
In order to dispatch an action dispatch needs to be called.
This is like effect already behaves.

In this issue I want to discuss if there is an easy way to provide an automatic migration path, or if a kind of other API should be introduced making it possible using the old and the new API in parallel.

[skydever]

hi there!

... some thoughts I have on this:

so the following needs to be changed with NgRx Ducks 9, is that correct?

this.myDuck.setValue(...) => this.myDuck.setValue.dispatch(...)
this.myDuck.setValue.action(...) => this.myDuck.setValue(...)

this.myDuck.setValue.action(...) could be marked as deprecated and still be available until 10 I guess? Maybe a rename of the Ducksify decorator could help? Ngrx Duck 9 will introduce a new decorator called DuckFacade (or something like that), and Ducksify will be deprecated ... this would aling with the strategy of Angular concerning breaking changes, to mark them as deprecated first ...

In theory it should be possible using schematics I guess, maybe we have to collect all Action annotated methods first to know what needs to be changed (if the info is not available in the AST), and in a second step find all that method calls and change them?

another path for such a migration could be a lint rule with an option for a fix, and then execute the linter using the --fix=true option? (like rxjs did it once)

• keep .action(..) to distinct between action creator and dispatching method
• mark the plain Action decorated calls as deprecated (if that is possible)
• the lint rule fix should change .setValue(...) to .setValue.dispatch(...) (if that is possible)
  ... with the release of NgRx 10:
• mark the .action() calls as deprecated
• the lint rule changes .setValue.action(...) to .setValue(...)

I don't have that much experience with AST and/or lint rule fixes, but this sounds very interesting and challenging ...

What do you have in mind? Do you think that the lint rule could be a simple solution?

[GregOnNet]

Good morning,

I need to read your comment a bit more in detail today but the first few lines are absolutely correct.
I also thought a lot about a new API adding more type-safety, too.
Yesterday, I came up with the idea below.
Now, the naming finds back to its Redux-roots, I think.

Simpler, breaking-change-free API

Definition

import * as selectors from './selectors.ts';

@InjectableSlice<T>({
  initialValue: {},
  providedIn: Module // check if exposing the injectable API adds benefit in terms of the initial bundle size
})
export class MySlice {
  select = bindSelectors(selectors);

  load = bindAction('Action without payload');
  search = bindAction<QueryParams>('Action having a payload');

  loadSuccess = bindAction(
    'action taking a case reducer',
    (slice: T, payload: TPayload) => {
       return { ...slice, payload }
    }
  );
}

export const reducer = reducerFrom(MySlice);
export const actions = actionsFrom(MySlice);

Usage

@Component(/* ... */)
export class MyComponent {
  constructor(private slice: Slice<MySlice>) {}
}

API	Description
bindSelectors	...same as bindSelectorGroup
InjectableSlice	Same as @ducksify. May have more possibilities configuring the IoC
Slice<T>	... same as Duck
bindAction	replaces effect and @action. bindAction combines NgRx's ActionCreator + Dispatcher. This should make the integration into the new effects a breeze.
reducerFrom	...same as before
actionsFrom	...extracts pure NgRx-action-creators

[GregOnNet]

//cc @Markus-Ende

[GregOnNet]

@skydever I really like the idea having tslint in place.
I wrote several rules before.

We can have a look into it.
If my proposal would work out, teams would not experience any breaking change for now.

It would be enough to use JSDoc and deprecate the "old" API (It is not that old actually... 🤣).

My basic intention is not to break anyone if it is not needed but I want keep improving the API little by little.

[skydever]

I like the new api 🎉 and an automated migration would be awesome, beside the possibility to still use the old one for now 👍

[GregOnNet]

Thank you so much for the feedback.

[Markus-Ende]

I also like this API, it feels more intuitive to have all those bind functions, especially the merged bindAction for effect and @action.

Why the rename from @Duck to @InjectableSlice?

[GregOnNet]

For now, there are 3 reasons:

1. InjectableSlice indicates that the decorator directly adds the annotated class to Angular's IoC
2. I also feel it is better to understand how ngrx-ducks should be used if the APIs are not merged into each other
3. The new API will be treated a little different in order to generate the actions etc. Some things will change under the hood.

[oemmes]

I think the propsed API would be much clearer then the current one.

The ditinction of effects and actions and the resulting effect.dispatch() vs. action() and return effect vs. return action.action() is really confusing if you do not know about it.

As I just stumbled upon difficulties using selectors with parameters (see #8) I think the new API should support that as well.

[skydever]

one more thing I have to add, maybe this is the right place ...

I was using static fields/methods at my Ducks to simplify the StoreModule setup, something like:

StoreModule.forFeature(MyDuck.featureName, MyDuck.reducer)

and I was also using the MyDuck.featureName at the selectors:

createFeatureSelector<MyState>(MyDuck.featureName)

but this seems not to be possible anymore (I am [very late] updating all my deps. at the moment). Do you think it is a bad idea to do it like this, and why?

... and another thing, it would be great to inherit the Duck from a base class that has some reusable logic, eg. the creation of the entityAdapter or things like that, that I then can use at the reducer functions. As far as I know a Duck is limited to action methods only. Would this be possible, and what are the current limitations?

Thank you!

[GregOnNet]

Hi Thomas,

sorry that the current version breaks you code.
The Duck was not intended to be used with static fields.

A Duck does not match to a feature but to a slice living in a feature.
So maybe it is not the best place to put a feature name into a Duck-class.

---

Concerning entityAdapter: We might need to set up another call here.
It would help me to see a comparison -> An extending Duck vs. a vanilla Duck.
I need to get a feeling for the benefits.

[skydever]

Hi @GregOnNet

Thx for the fast reply, I already adopted the code 👍

A Duck does not match to a feature but to a slice living in a feature.
So maybe it is not the best place to put a feature name into a Duck-class.

Maybe I missed something (I started using ngrx with the Ducks because I did not like the boilerplate of plain ngrx (that was reduced quite heavily with the new action creators), so my plain ngrx experience is very low). So you mean a Duck should always be part of a feature using a ActionReducerMap or combineReducers setup? My Ducks are sometimes part of a feature, but I also have them as a standalone feature using StoreModule.forFeature() as described above. For me it was quite handy doing it the static way concerning ide auto-import at the module setup, for all usages (ActionReducerMap, standalone feature ...). Actually, I had the feature name in the file of the store interface and just pointed to it from the Duck, and the selectors also pointed to the store interface file (to avoid circ. deps between selectors + duck).

---

With the new API it should be possible to create a base Duck that contains all CRUD methods as actions (using a protected prefix and/or entityName to create the action types), and on top of that at the extending Duck it should be possible to add actions that do some special modifications, but for basic CRUD you don't have to do anything. You can use the entityAdapter from the base, add other utility methods to the base class ... going into that direction, but not just for entity CRUD operations ...

[skydever]

I think it would be great if there are no restrictions, even DI into a Duck class... in my opinion this would enhance the Ducks a lot ...

[skydever]

... maybe there is a better functional programming approach, but in terms of classes and inheritance this would be awesome ...

[GregOnNet]

I will take everything into account.
You are right, destroying the basic functionality of the class itself should not happen. I will have a look why static fields do not work anymore...

Thanks for your input.

Concerning entity adapter. What do you think about the following API.

class EntityAdapter {
  addOne() {}
  addAll() {}
  addMany() {}

  // ...
}

@InjectableSlice({})
export class MySlice extends EntityAdapter {
  loadSuccess = bindAction(
    'action taking a case reducer',
    (slice: T, payload: TPayload) => {
       return addOne(payload, slice)
    }
  );
}

[GregOnNet]

@skydever I now remember that we discussed the EntityAdapter a bit differently before.
Your idea was having the action prepared for every adapter method, right?

[GregOnNet]

The progress of the bindAction-API can be tracked here: #9

[skydever]

@GregOnNet yes, that was the idea back then - I also created a @EntityDucksify() decorator on top of @Ducksify() to do so, passing a actionTypeSource and entityName to automatically create the action types - but I had to play around with the prototype to get it working, and luckily the types worked out ... it is very useful for some of our projects, but I think it is not the most stable solution, would break very easyily ... doing it in using real inheritance without tricks would be very nice ... I think with the new API this may could be done in a better, more stable way - can't wait to play around with it then 😃

[GregOnNet]

Closing this issue, for now.
All the features of the next release are documented in issue #9.