Skip to content

fix(persisted-state): do not make complex objects reactive #270

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

43081j
Copy link
Contributor

@43081j 43081j commented Jun 4, 2025

We currently make all non-Date objects "reactive" in that we wrap them in a Proxy. We use this to be able to increment the persisted version and write the new serialized value.

However, this leads to a lot of funkiness when trying to Proxy things like Set, Map, etc.

This change dumbs the logic down to only Proxy plain objects.

This means the following will not trigger a write to storage anymore:

const value = {
  foo: new SomeClass()
};
const state = new PersistedState(key, value, {serializer});

state.current.foo.prop = 200; // no longer writes
state.current.foo = new SomeClass(200); // writes

This should fix #268

We currently make all non-Date objects "reactive" in that we wrap them
in a `Proxy`. We use this to be able to increment the persisted version
and write the new serialized value.

However, this leads to a lot of funkiness when trying to `Proxy` things
like `Set`, `Map`, etc.

This change dumbs the logic down to _only_ `Proxy` plain objects.

This means the following will not trigger a write to storage anymore:

```ts
const value = {
  foo: new SomeClass()
};
const state = new PersistedState(key, value, {serializer});

state.current.foo.prop = 200; // no longer writes
state.current.foo = new SomeClass(200); // writes
```
Copy link

changeset-bot bot commented Jun 4, 2025

🦋 Changeset detected

Latest commit: 623f2d8

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
runed Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Contributor

github-actions bot commented Jun 4, 2025

built with Refined Cloudflare Pages Action

⚡ Cloudflare Pages Deployment

Name Status Preview Last Commit
runed ✅ Ready (View Log) Visit Preview 623f2d8

@TGlide
Copy link
Member

TGlide commented Jul 14, 2025

Thank you for the changes! IMO we should probably document this too in the .md file 👀

@43081j
Copy link
Contributor Author

43081j commented Jul 14, 2025

@TGlide this should be sorted now

i updated the docs and also added support for arrays

i caught up from main also

lint fails but because some files outside this PR are not formatted (so i guess they're like that in main?)

@MRocholl
Copy link

Just stumbled across this PR. Is this also related to PersistedState of a SvelteMap or SvelteSet for instance? Or is that already supported somehow?

@43081j
Copy link
Contributor Author

43081j commented Jul 16, 2025

With this pr, they will expect that the serialiser you use is capable of serialising them. But they won't be deeply reactive, i.e. adding a value to a svelte set won't trigger a persist.

But why would you be trying to persist complex objects like this? If you have a good real world example, it would help a lot

@MRocholl
Copy link

MRocholl commented Jul 21, 2025

Ive since moved to simple objects. But I am keeping track of added objects and modified objects via a Map. That was the original use case for me. This might simply be a matter of understanding this library better. I kinda expected to be able to throw any reactive state into PersitentState.

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.

PersistedState using a Set or Map throws
3 participants