-
Notifications
You must be signed in to change notification settings - Fork 406
feat(store): implement ActionDirector
#2329
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
Conversation
0239ed1
to
86e0f6c
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
00eca4e
to
3d2d0a0
Compare
3d2d0a0
to
25deb53
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you so much for trying this out, but I'm not a fan of this approach.
It doesn't solve the complexity around injection context. And the queue seems a bit arbitrary. My thought along these lines was that we would be queueing the action handlers on a static queue, so that attachAction
can be called independant of an injection context.
I think that I am solidly leaning away from the "magic" 'call from anywhere' function, and towards the more explicit call to a member of a service.
I'll unpack my thoughts in the next comment on this PR...
I think that the following API would make things much simpler and more explicit: const actionDirector = inject(ActionDirector);
// somewhere appropriate in the code
actionDirector.attachAction(
CountriesState,
AddCountry,
(ctx: StateContext<string[]>, action: AddCountry) => {
ctx.setState(countries => [...countries, action.country]);
}
); and to build on this example with more a complex usage and side effects: const actionDirector = inject(ActionDirector);
const countryRepo = inject(CountryRepo);
// somewhere appropriate in the code
const detachMe = actionDirector.attachAction(
CountriesState,
AddCountry,
async (ctx: StateContext<string[]>, action: AddCountry) => {
// Note: this is not run inside an injection context, just like in the states
ctx.setState(countries => [...countries, action.country]);
await countryRepo.Add(action.country);
}
);
const destroyRef = inject(DestroyRef);
destroyRef.onDestroy(() => detachMe()); We could also provide another function ( const actionDirector = inject(ActionDirector);
const countryRepo = inject(CountryRepo);
// somewhere appropriate in the code
const stopEffect = actionDirector.effect(
AddCountry,
async (action: AddCountry) => {
// Note: this is not run inside an injection context, just like in the states
await countryRepo.Add(action.country);
}
);
const destroyRef = inject(DestroyRef);
destroyRef.onDestroy(() => stopEffect()); With respect to the naming of these functions, I have thought of a few options:
What do you think? |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
25deb53
to
fb0bf6b
Compare
Good points. I think I'm leaning towards I also think that the signature for the function should require the state model type information (to be able to provide to the StateContext), and, with that in mind, maybe requiring a StateToken as opposed to a state class as the first parameter would push towards the more type safe API. |
fb0bf6b
to
b896ec2
Compare
packages/store/tests/build-action-handler/build-action-handler.spec.ts
Outdated
Show resolved
Hide resolved
packages/store/tests/build-action-handler/fixtures/add-country-action-handler.ts
Outdated
Show resolved
Hide resolved
|
||
- Add action handlers conditionally based on runtime conditions | ||
- Add action handlers for lazy-loaded modules | ||
- Add temporary action handlers that can be removed later |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Add action handler to fine-grain control when an action is completed
|
||
- It gives you the ability to make changes to state from your handler | ||
- It participates in the standard [actions life cycle](./actions-life-cycle.md) | ||
- The result of the handler will affect the completion result of the action |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe add an example?
(i.e. if the action handler is
async
, dispatched action will be "COMPLETED" once it finishes)
efb9939
to
6922684
Compare
6922684
to
8df1b09
Compare
attachAction
ActionDirector
The `ActionDirector` allows you to attach action handlers to a state at any point after initialization and gives you the ability to detach them when no longer needed.
8df1b09
to
d8e1bf5
Compare
The
ActionDirector
allows you to attach action handlers to a state at any point afterinitialization and gives you the ability to detach them when no longer needed.