[ ] Regression (a behavior that used to work and stopped working in a new release)
[ ] Bug report
[ ] Performance issue
[ ] Feature request
[ ] Documentation issue or request
[x] Support request
[ ] Other... Please describe:
Assuming I have a data payload coming from signalR, that calls this method:
private applySignalrPayload(store, entityId, payload): void {
switch (store) {
case AkitaEntityStores.DeviceTelemetry:
this.deviceTelemetryService.upsert(entityId, payload);
break;
case AkitaEntityStores.DeviceAlerts:
this.deviceAlertService.upsert(entityId, payload);
break;
default:
this.logger.warn('No matching Akita Entity Store was found to update.');
}
}
Alternatively, how is that better or worse than this:
private applySignalrPayload(store, entityId, payload): void {
runStoreAction(store, StoreActions.UpsertEntities, {
payload: {
data: payload,
entityIds: entityId
}
});
}
The only different is that using runStoreAction requires putting the action i.e. upsert on the store and calling super.upsert. Whereas injecting the service requires putting the action on the service.
What are the pros/cons of using runStoreAction vs injecting the service and updating this way? They accomplish the same thing so I'm wondering what the use case of runStoreAction is when we can inject the services instead.
The runStoreAction simplifies interactions with event-based APIs, that's all.
@NetanelBasal
What if the incoming websocket/SignalR payload specifies the store name and you don't have store instance? Are you supposed to do the switch/case mentioned above?
Why didn't you leave backwards compatibility in the new version?
@xbaun we lost the flexibility I wanted in the first place regarding the store name. Can you support both a string and instance as @hakimio said, please?
Why didn't you leave backwards compatibility in the new version?
I agree that the flexibility in comparison to the previous implementation has been lost. The basic dilemma was that runStoreAction took on the role of a general purpose action creator and this, combined with the desired flexibility, caused two problems:
upsert operation are quite complex and cannot be mapped to such a generated action object and keep all type information.What if the incoming websocket/SignalR payload specifies the store name and you don't have store instance
The current implementation assumes that the data types and thus the target store are determined before the action is executed. The runStoreAction function expects the store class as first parameter not an instance. This means that when server data is received at the client, a store class should be mapped to it and then an appropriate action fired:
if (data.storeName === 'BooksStore')
runEntityStoreAction(BooksStore, EntityStoreAction.RemoveEntities, remove => remove(data.payload.id));
A fix would be to rollback the runStoreAction changes and reuse the previous code. This maintains flexibility, but without support for typescript's static type checking analysis. With restrictions in the implementation of the upsert action.
Finally, the question is whether external data should first be validated and thus casted to the correct type (determining the store class, state properties and so on ...) before actions are executed through runStoreAction.
Actions can be executed against a wrong store without typescript showing a warning/error (the generated state from the action's payload does not match the store state)
The new implementation doesn't really solve that. Following will still execute the action against the wrong store:
if (data.storeName === 'AppleStore')
runEntityStoreAction(PearStore, EntityStoreAction.RemoveEntities, remove => remove(data.payload.id));
Moreover, you are making the code needlessly more complicated by requiring to add extra logic:
switch (store) {
case 'foo':
runEntityStoreAction(FooStore, ...);
break;
case 'bar':
runEntityStoreAction(BarStore, ...);
break;
case 'baz':
runEntityStoreAction(BazStore, ...);
break;
}
instead of just:
runEntityStoreAction(store, ...);
the question is whether external data should first be validated and thus casted to the correct type (determining the store class, state properties and so on ...) before actions are executed through runStoreAction
If you trust the data source and it is already in correct format, why would you need to validate or transform it?
Anyway, I would vote for leaving the old runEntityStoreAction() definition, but it might be best if @NetanelBasal would decide or it would be a community vote.
@xbaun, we've added the runStoreAction to simplify the interaction with events-based APIs.
We can support both a string and a store, no? If someone uses the string version, they'll not benefit from type safety.
The new implementation doesn't really solve that. Following will still execute the action against the wrong store:
My example was perhaps too simple, because only the id is needed for this particular action. Take, for example:
runEntityStoreAction(BooksStore, EntityStoreAction.UpdateEntities, update => update(data.payload.ids, data.payload.stateUpdate));
runEntityStoreAction(BooksStore, EntityStoreAction.UpsertEntities, upsert => upsert(data.payload.ids, data.payload.stateUpdate, (id, newState) => ({ id, ...newState, price: 0 })));
Here the arguments of the action has to match the new partial state to update the store correctly.
If you trust the data source and it is already in correct format, why would you need to validate or transform it?
For me it's more about preventing bugs (not only now, but also in the code evolving over time) because of wrong typings, not if the data source is trustworthy. But as I mentioned before, this is a fundamental design decision, and I am open to any solution, including a rollback to the previous code.
We can support both a string and a store, no? If someone uses the string version, they'll not benefit from type safety.
I used the store class to automatically infer the action and its arguments. With only a store name this would not work, because no static type mapping is provided between the name and the available stores. Implementing both in the same manner is probably not that easy.
I really want to support both, so we need to find a solution. If not, in this case, I prefer the flexibility for the event-based APIs.
I'm happy to see all the activity this issue has generated. The only thing I didn't get a clear answer from was this:
Alternatively, how is that better or worse than this:
With an event based API, how is the runStoreAction simpler or more flexible than my example switch case?
private applySignalrPayload(store, entityId, payload): void { switch (store) { case AkitaEntityStores.DeviceTelemetry: this.deviceTelemetryService.upsert(entityId, payload); break; case AkitaEntityStores.DeviceAlerts: this.deviceAlertService.upsert(entityId, payload); break; default: this.logger.warn('No matching Akita Entity Store was found to update.'); }}
The runStoreAction is some kind of a convenient function:
Thanks @xbaun
Awesome, thanks! @xbaun,@NetanelBasal and @hakimio
One more thing. When I use runStoreAction is there any way I can still specify my actions on the service? I guess like runSerficeAction instead of store action.
My akita app is not working anymore when upgraded to angular version 10
and also upgraded akita to
@datorama/akita version 5.2.5
Help me! Now!!!
App error is complaining about line 4 and line 8 in this file:
https://github.com/silverstory/akita-socketio/blob/master/src/app/livefeed-list.service.ts
Link to app
https://github.com/silverstory/akita-socketio