Question: Upgrading ngrx/store to utilize new creator functions
Opened this issue · 10 comments
This is purely a question about whether upgrading ngrx-forms core to make use of the action creator/reducer creator functions provided by ngrx. It's something I'm exploring currently, but didn't want to go about updating if it's not something you desire. It would be quite a big change with hopefully no real functionality lose and it would keep the library in line with ngrx.
Curious to know your opinion on the matter, if you think it's a good idea I'd consider starting the work.
Hmm, yeah, this is something I was thinking about for some time. Obviously this would be a breaking change that would also bump the peer dependency requirement for ngrx to 8. So far I got away with keeping the lib compatible with 7. Since I'd like to prevent having to backport bugfixes etc I'd only want to make this change if the number of users of ngrx that are using version 7 or lower is very low. Since I am not really working in the Angular ecosystem right now I am not sure what the situation looks like. What is your perception? Is the majority of the user base already on ngrx 8?
- ngrx v8 has been out for more than a year - that's a big time frame in scope of Angular ecosystem. Eventually, devs have to update their ngrx if they want to use it on newer angular (v8, v9, v10, ...).
- IMO this library is so stable there is no reason to be afraid of backporting bugfixes as there will be none.
So, I would say it's safe to do this breaking change.
Side question: where did you @MrWolfZ switch to? 😅
Fair enough, feel free to create a pull request for switching to the new functions. I'm also curious if that will positively affect the bundle size since I feel that the action classes don't minimize that well. Please make sure to work on the develop branch in your fork and then also target that branch in the PR.
Regarding where I switched to: professionally I am currently working on infrastructure instead of UI, i.e. setting up CI/CD pipelines, configuring servers and Kubernetes clusters etc. Privately, I use mostly React for UI projects (shameless plug: using my simplux state management library) and recently have started looking into Svelte.
@tashoecraft are you willing to take on this?
@dzonatan yeah I’ll take a stab at it.
So there's a lot of little questions that are going to come up as creatorFunctions are going to change the semantics of how things are named and stored, less so the actual functionality. I'll try and post them up before I get too deep into working on anything in order to not waste too much time.
To start with actions let's look at SetValueAction
export class SetValueAction<TValue> implements Action {
static readonly TYPE: 'ngrx/forms/SET_VALUE' = 'ngrx/forms/SET_VALUE';
readonly type = SetValueAction.TYPE;
constructor(
public readonly controlId: NgrxFormControlId,
public readonly value: TValue,
) { }
}
with creator functions it's
export const SetValueAction = createAction(
'ngrx/forms/SET_VALUE',
props<{controlId: NgrxFormControlId, value: any}>()
)
Now what's going to be a slightly annoying api change is each time you want to call SetValueAction you'll need to write it out as
SetValueAction({controlId: 'controlId', value: value})
The conversion to an object as a param host its cost in terms of ergonomics IMO.
Now for accessing the type of the action the codebase currently does things like
SetValueAction.TYPE
in my own projects, I've typically done it where the types are stored as an exported enum. We can convert the existing variable of ALL_NGRX_FORMS_ACTION_TYPES to accommodate here
export enum ALL_NGRX_FORMS_ACTION_TYPES {
SetValueActionType = 'ngrx/forms/SET_VALUE',
}
Then inside the action and whenever one wishes reference the type you can just do
ALL_NGRX_FORMS_ACTION_TYPES.SetValueActionType,
but it's a bit more verbose due to that variables length.
If either of you has any opinions to the contrary here I'm all eyes, otherwise, I'll continue.
I personally don't mind that little bit more verbosity due to the payload being objects.
Regarding the .TYPE, and using an enum for it, the main motivation for doing that is less duplication and better type inference in reducers. What is the type of action.type when doing createAction(ALL_NGRX_FORMS_ACTION_TYPES.SetValueActionType, ...)? I would think it is ALL_NGRX_FORMS_ACTION_TYPES while it should be the string literal 'ngrx/forms/SET_VALUE'. However, maybe if all the internal reducers are refactored to use the new ngrx style that doesn't matter, since the type inference for action reducers doesn't depend on the .type property anymore but instead the whole action creator function. If that is the case, I wouldn't mind putting them in the enum, since at least in the internal code base you would basically never access the enum members anyways.
Exactly as the reference to the .type property is no longer as important. I've been chipping away at it, my only concerns so far are the extent of forcing people to update all their actions to use objects as opposed to passing in multiple arguments, but that could potentially be mitigated by an update schematic I believe. Also the extent that I'm unable to pass as indepth type information as was previously there. Not sure if that's an actual problem, but the use of any has definitely increased. Will probably rely on someone more knowledgeable in types to take a look once I have a PR up.
You can use the createAction-fnc with a creator-fnc.
For example your code can look like this
export const SetErrorsAction = createAction(NGRX_FORMS_ACTION_TYPES.SetErrorsActionType, (controlId: NgrxFormControlId, errors: ValidationErrors) => ({controlId, errors}));
setErrorsReducer(INITIAL_STATE, SetErrorsAction(FORM_CONTROL_ID, errors)); instead of setErrorsReducer(INITIAL_STATE, new SetErrorsAction(FORM_CONTROL_ID, errors));. Not much different only 'new' is dropped and of course you have to replace every *.TYPE with *.type
ngrx-forms-6.3.4.tgz decreased to 377kb (down from 554kb), Angular 11, ngrx-store 11-beta.1.