angular-architects/ngrx-toolkit

proposal: withLocalStorage/withSessionStorage

Closed this issue ยท 15 comments

ducin commented

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.

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 commented

@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)?

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.

@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.

@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 commented

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

ducin commented

@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.

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

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,
  })
);

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.

I actually think the PR should get merged it seems perfect

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 :) )

@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 ๐Ÿ˜‚)

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.

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