Skip to content

proposal: withLocalStorage/withSessionStorage #11

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

Closed
ducin opened this issue Feb 6, 2024 · 15 comments
Closed

proposal: withLocalStorage/withSessionStorage #11

ducin opened this issue Feb 6, 2024 · 15 comments

Comments

@ducin
Copy link

ducin commented Feb 6, 2024

Hi Angular Architects!

Here's my proposal for a custom signalStoreFeature to be included within this library. I wanted to make my own library, but it makes much more sense to join forces. If you're open for contributions/PRs from outside...?

The idea behind this web storage synchronization is:

  • per store (granular, not global like)
  • redux-independent
  • only signals are taken into account, not computeds.
  • private withWebStorage implementation serves: withLocalStorage/withSessionStorage
withLocalStorage(key: string, {
  validatorFn: (obj) => void, // required validator will be less troublesome in the long run
  serializer?: (obj) => string, // defaults to JSON.stringify
  deserializer?: (string) => obj // defaults to JSON.parse
})

which does the following:

  • onInit -> deserialize the storage content if it exists, iterate over signal properties and set them
  • whenever an update happens on a signal, it's being serialized and dumped to web storage: withHooks / onInit / effect -> reads the state, serializes it and pushes. This establishes synchronization

Please let me know what you think. If the idea is ok, I'll work on the PR.

@rainerhahnekamp
Copy link
Collaborator

Hiho, sure thing. To me, the only question is about the validatorFn.: What should that do, and why do you think the user has to provide an implementation?

@ducin
Copy link
Author

ducin commented Feb 6, 2024

@rainerhahnekamp form my experience, working with any web storage is vulnerable to issues with versioning serialized data structures sooner or later. Example:

  • a project starts with structure A
  • within some time, it's extended/refactored to structure B
  • some users could be left with structure A because of reasons... (e.g. they used the app before the release, whatever)
  • we don't want unexpected errors to pop up.

Anyway I'm open to suggestions - what is your take - make it optional? Or a different approach (dunno, migrating)?

@Jordan-Hall
Copy link

Jordan-Hall commented Feb 6, 2024

I think adding withStorage would be a better approach, default it to session maybe, but allow you to pass in any storage interface https://developer.mozilla.org/en-US/docs/Web/API/Storage its a simple API so it's extendable too.

I also think make validatorfn optional too.

@rainerhahnekamp
Copy link
Collaborator

@markostanimirovic: as far as I know the NgRx team has plans to come up with such a feature in the future as well. could you please comment here, how concrete those plans are?

I guess @ducin doesn't want to spend his time implementing it and - all of a sudden - an official pops up.

@rainerhahnekamp
Copy link
Collaborator

@ducin yeah, I'd say to get started very quickly that we provide a validator function by default. We can still discuss what that one should do.

I also think that the use cases you mentioned along different strategies should definitely show up in the documentation.

@ducin
Copy link
Author

ducin commented Feb 6, 2024

oh definitely!
If there's plan to make an official one, I'd be also happy to help with it @markostanimirovic.

@ducin
Copy link
Author

ducin commented Feb 6, 2024

@Jordan-Hall do you mean pass the global object (not "interface"), e.g. localStorage?

Subjectively, I don't have strong preference with withStorage(localStorage, ...) vs withLocalStorage(...) - it's clear what they do both ways.

@Jordan-Hall
Copy link

@ducin correct sorry pass in the object withStorage(localStorage) but for types use the storage interface.

@markostanimirovic
Copy link

Hey 👋

That's true, we're considering introducing this plugin to the core library, but it's still not finally confirmed. ngrx/platform#3997

Here is the implementation of the withStorageSync feature from the initial SignalStore prototype: https://github.com/markostanimirovic/ngrx-signal-store-playground/blob/main/src/app/shared/storage-sync.feature.ts

Note: To make this feature SSR-friendly, the storage factory needs to be passed as an input argument instead of a storage:

export const CounterStore = signalStore(
  withState({ count: 0 }),
  withStorageSync({
    key: 'counter',
    // if not passed, `localStorage` is used as the default
    storage: () => sessionStorage,
  })
);

@bohoffi
Copy link
Contributor

bohoffi commented Feb 7, 2024

When I created ngrx/plattform#4180 a little over motivated at the end of 2023 @rainerhahnekamp suggested opening a PR for ngrx-toolkit which can be found here ngrx-toolkit#7.
If adding this functionality to ngrx-toolkit is still on hold while an official plugin is being discussed - I'd close my pr.

@Jordan-Hall
Copy link

I actually think the PR should get merged it seems perfect

@rainerhahnekamp
Copy link
Collaborator

Yes, so I'd say that if @bohoffi and @ducin want to take on the risk that it might become redundant, we can add it to the toolkit.

So if you both agree (@Jordan-Hall of course you are invited to join here as well), we should maybe start discussing the API.


PS: @bohoffi thousand apologies but I didn't see your PR. I guess it was due to the day. "New Year's Eve" is my birthday and there is usually a trend that everybody else celebrates on that (but maybe for other reasons :) )

@bohoffi
Copy link
Contributor

bohoffi commented Feb 8, 2024

@rainerhahnekamp I'm not in a hurry as I'm using the implementation as provided via the PR in a private project already.
Even if a merge will become obsolete in the future due to an official plugin a discussion about the API could offer some value somewhere.

(Well not everyone's having such a big bang birthday party 😂)

@rainerhahnekamp
Copy link
Collaborator

With version 0.0.7, the sync storage extension is now available. @ducin, if you want to add or modify some of the features... we would be ready.

@rainerhahnekamp
Copy link
Collaborator

Closing this issue since no further comments for 10 days and the version of @bohoffi is already available.

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

No branches or pull requests

5 participants