Skip to content
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

Move id creation out of actor/reducer #37

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

scally
Copy link

@scally scally commented Aug 29, 2020

When following along with the demo, I noticed that my ids were being recreated each time the app started, and they were never persisted.
I realized that this is because, like with redux, you can't define your ids in your reducers. They have to be part of the payload in the action creator from the beginning.

This was the most straightforward fix, although it's not as tidy.

When following along with the demo, I noticed that my ids were being recreated each time the app started, and they were never persisted.
I realized that this is because, like with redux, you can't define your ids in your reducers. They have to be part of the payload in the action creator from the beginning.

This was the most straightforward fix, although it's not as tidy.
@ncthbrt
Copy link
Owner

ncthbrt commented Aug 29, 2020

Hi @scally. Nact doesn't have the problem that redux does in that regard. The persist function can save any arbitrary data, not just the exact message being passed in, which means that it is able to modify the create contact messages to include the id and save them. I believe you have found a legitimate bug in the example code, but it'd be better to solve it in the persist section:

msg.id = newContact.id;
if(!ctx.recovering) { await ctx.persist(msg); }

@scally
Copy link
Author

scally commented Aug 31, 2020

@ncthbrt Thanks for taking a look. I could have sworn I tried something like that already and it didn't work, but I'll look again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants