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

feat: add PersistedState #113

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

Conversation

Not-Jayden
Copy link

@Not-Jayden Not-Jayden commented Jul 6, 2024

Creates reactive state that is persisted and synced across sessions and browser tabs. Inspired by https://github.com/joshnuss/svelte-persisted-store

See docs: https://not-jayden-persisted-state.runed.pages.dev/docs/utilities/persisted-state

Copy link

changeset-bot bot commented Jul 6, 2024

🦋 Changeset detected

Latest commit: 41f9495

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

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

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

@Not-Jayden Not-Jayden changed the title init persisted state feat: Persisted State Jul 6, 2024
@Not-Jayden Not-Jayden changed the title feat: Persisted State feat: add PersistedState Jul 6, 2024
Copy link
Contributor

github-actions bot commented Jul 6, 2024

built with Refined Cloudflare Pages Action

⚡ Cloudflare Pages Deployment

Name Status Preview Last Commit
runed ✅ Ready (View Log) Visit Preview 41f9495

@Not-Jayden Not-Jayden marked this pull request as ready for review July 6, 2024 11:55
Copy link
Author

@Not-Jayden Not-Jayden left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Self-review notes:

  • Maybe just rename to Persisted? Simpler and consistent with the svelte-persisted-store package naming
  • Maybe add a reset() method like svelte-persisted-store? Very much a nice-to-have addition useful for only a small set of use cases, with the trade-off that it comes with a small memory overhead of needing to store the initial state. Can ultimately be achieved without needing the explicit method either way.
  • svelte-persisted-store also contains the following optional props for error handling and data transformation that I omitted. Don't personally feel they are necessary, though they could easily be added in a later PR if desired.
  onWriteError: (error) => {/* handle or rethrow */}, // Defaults to console.error with the error object
  onParseError: (raw, error) => {/* handle or rethrow */}, // Defaults to console.error with the error object
  beforeRead: (value) => {/* change value after serialization but before setting store to return value*/},
  beforeWrite: (value) => {/* change value after writing to store, but before writing return value to local storage*/},

@huntabyte
Copy link
Member

huntabyte commented Jul 13, 2024

I haven't had a chance to give this a thorough review, but I love what I see.

Regarding the name, I think Persisted is cleaner, though I'll defer to the others if dropping the State makes what it does less obvious.

For something like this, I wonder if we can take it a step further and accept a custom Storage type rather than just the custom serializer/deserializer. We can include adapters for session and local storage out of the box while allowing users to BYOS any storage system they wish as long as it follows our Storage interface.

I'm currently imagining a situation where we could leverage this along with something like Supabase or any real-time DB to bind specific values. If something changes in the DB, it updates, and if something changes that value in the app, it updates the DB. Having a flexible StorageAdapter would be sick for something like this!

@Not-Jayden
Copy link
Author

Not-Jayden commented Jul 15, 2024

I haven't had a chance to give this a thorough review, but I love what I see.

Regarding the name, I think Persisted is cleaner, though I'll defer to the others if dropping the State makes what it does less obvious.

For something like this, I wonder if we can take it a step further and accept a custom Storage type rather than just the custom serializer/deserializer. We can include adapters for session and local storage out of the box while allowing users to BYOS any storage system they wish as long as it follows our Storage interface.

I'm currently imagining a situation where we could leverage this along with something like Supabase or any real-time DB to bind specific values. If something changes in the DB, it updates, and if something changes that value in the app, it updates the DB. Having a flexible StorageAdapter would be sick for something like this!

@huntabyte thanks! Agree the adapter might be good for future extensibility, I've had a crack at it in this branch: #118

Seems to be working fine but will sanity check it over the weekend to be sure and update docs if all looks good. If you have any feedback in the meantime it would be appreciated to make sure I don't sink too much time into going the wrong direction :)

@huntabyte
Copy link
Member

Going to give this a review this week, I promise @Not-Jayden!

Thanks for your patience!

@huntabyte
Copy link
Member

Discussing the storage adapter PR with the other maintainers as we speak! Hopefully we can polish it and get this shipped sooner rather than later 😃

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