-
Notifications
You must be signed in to change notification settings - Fork 343
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
Store Update - merges current and previous state values. #661
Comments
The update method will merge the current state with the new one. You can use the setState method to override it. |
That does do exactly what I needed, Thank you. With the method being marked as @internal : Would you be open to making (or accepting a pull request for) a public version of this functionality available to address the use case I've outlined above. Something along the lines of:
with a supporting hook:
Set may not be the best name for it maybe replace or something else? |
I prefer to add options to the update method. |
Something like:
The only concern I would have with boolean flags in an options object is adding conditional complexity to a function call e.g. what if I want different hook behaviour for this do we start doing something like this:
Maybe it would be better to have clean simple method signatures and abstract away some of the same repeated code.
This would keep the public api transparent and explicit, removing any ambiguity? |
Ok, let's go with option two |
Ok, I'll put together a pull request later today. |
Created a public method on the akita store that will completely replace the current state with the new provided state and not just update the value. Closes salesforce#661
Renamed the new set method to overwrite in an effort to avoid conflicts with the entityStore method set. Closes salesforce#663 PR salesforce#661
Removed redundant local functions in favour of inlining. Simplified the documentation example for overwrite() Closes salesforce#661 PR salesforce#663
Hey guys! I ran into this issue today which was unexpected. It would seem the PR became stale and given the passage of time, may no longer be relevant/required? Would you still suggest using the internal Thanks! |
I'm submitting a...
Current behavior
When calling store.update only the specified properties are being replaced.
If I have two possible states I want to represent in the Todos store
I intialize my todos state to a TodosBasicState
When I update the state using the following method I get a polluted state object that is a combination of the old state and the new values.
State object as json before update:
State object as json after update:
As you can see the the _tag and altInfo properties have been updated as expected but the rest of the state has not been removed from the previously.
The following line is the cause in store.ts. The current and new state are both being combined in a spread operation. I'm sure this is being done for a reason I'm ignorant of but is this side effect intentional?
https://github.com/datorama/akita/blob/721243c814b8835a0bec113a686c5a1400dc2f2a/libs/akita/src/lib/store.ts#L256
Expected behavior
I would expect that when calling the update method I get only the specified properties in the state and all old properites are dropped.
Expect this:
to turn state from:
to this:
If I wanted to keep values from the previous state I would expect to have to include them in the update callback using a spread or explicitly.
e.g.
Minimal reproduction of the problem with instructions
https://stackblitz.com/edit/akita-todos-app-pyf2xv?file=src/app/todos/state/todos.query.ts
Click update to perform the above update method and click reset to reset the store back to the initial method.
What is the motivation / use case for changing the behavior?
I want to be able to update the state without previous state values polluting the new state. If there is a better way to achieve this any advise is welcome?
Environment
The text was updated successfully, but these errors were encountered: