Noitidart / valtio-persist

MIT License
71 stars 5 forks source link

Actions lost when using persisted state? #1

Open GollyJer opened 2 years ago

GollyJer commented 2 years ago

Hi! I think this is expected behavior but I thought I'd ask.

Is it expected behavior for actions to be destroyed when using actions in state.

I have a codesandbox showing the behavior here. If you go to 👆, and click the Persisted Thing buttons before refreshing the page, it will work. But it you refresh the page and try pressing the buttons again then it will throw persistedThing.add is not a function error.

I was hoping to keep access to all functionality under the state object but it looks like I'm going to have to break apart actions from state if I want to use persistence.

Is this a correct assumption?

Noitidart commented 2 years ago

Ah yes, I was coming from redux land so I hadn't put non-serializable data into store yet. I'll note this in the readme. Thanks for getting involved with this lib, having others to brainstorm with makes things so much better! If you have any ideas on how to keep non-serializable in state too while persisting it, I could try hammering it out some time after the error handling stuff.

I was thinking, if the non-serializable was in initialState then after rehydrating, those things could be brought over, what do you think? Would this solve all usecases of non-serializable in state?

GollyJer commented 2 years ago

Now that you mention it. The "thing" files are exporting a proxy, not a primitive. I'm not actually sure how it's working correctly to serialize those.


I was thinking, if the non-serializable was in initialState then after rehydrating, those things could be brought over, what do you think? Would this solve all usecases of non-serializable in state? That does seem like a decent approach.

Here's a good article on the topic.

I do think Map/Set would be good to support for persist. I've got my eye on this SO answer. It doesn't have too many compromises.

const serialMap = JSON.stringify(Array.from(myMap.entries()))
const myRestoredMap = new Map(JSON.parse(serialMap))

Anyway... The rule for me, even though we aren't in a redux world, is to only use serializable data in state. From a Valtio perspective this means you can't use the Actions Defined in State pattern.

Noitidart commented 2 years ago

Now that you mention it. The "thing" files are exporting a proxy, not a primitive. I'm not actually sure how it's working correctly to serialize those.

I think this works because proxy behaves just like an object in all things, JSON.stringify, Object.keys, enumeration etc. So that's awesome thanks for sharing that discovery with me, I would have never known.

I do think Map/Set would be good to support for persist. I've got my eye on this SO answer. It doesn't have too many compromises.

const serialMap = JSON.stringify(Array.from(myMap.entries()))
const myRestoredMap = new Map(JSON.parse(serialMap))

Map/Set support would be awesome. We would have to hold something in the persisted data to note that it is a map/set though. As the result of serialMap does not have any hint that it's a map. :(

Noitidart commented 2 years ago

Hey @GollyJer - how is your experience been with this. I haven't been able to add in error handling yet, like if a write fails it needs a retry mechanism and after all retries it should inform the dev so he can log it to his error logger or something.

Currently it's got some basic error handling with no retries.

GollyJer commented 2 years ago

Hey! I ended up making a whole new version based on your ideas... but, ultimately had to abandon Valtio for now because it made our memory management issues worse. 😿

Noitidart commented 2 years ago

Ah thanks for sharing, I was suspecting Valtio was having some memory issues too with lots of data but was just a hunch. Your comment really helps. If you could share anything about those memory issues, that would be awesome, I can try to bring up with Valtio author.