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

PersistedState: Add a way to validate values #151

Open
webJose opened this issue Dec 1, 2024 · 4 comments
Open

PersistedState: Add a way to validate values #151

webJose opened this issue Dec 1, 2024 · 4 comments
Labels
enhancement New feature or request help wanted Open to contributions from the community.

Comments

@webJose
Copy link
Contributor

webJose commented Dec 1, 2024

Describe the feature in detail (code, mocks, or screenshots encouraged)

I have already gone through this with svelte-persisted-store: Software evolves, and so the shapes of the data that is stored in local/session storage. Today, I store a Boolean, but tomorrow I may add a third option, so now I want to store an enum. The store should have a way to validate stored data and potentially upgrade it/fix it. Yes, fixing should be an option: People can mess with the value by hand.

What type of pull request would this be?

Enhancement

Provide relevant links or additional information.

*Don't know the difference between "Enhancement" and "New Feature".

@webJose webJose added the enhancement New feature or request label Dec 1, 2024
@huntabyte
Copy link
Member

This would be an enhancement!

So you're thinking of providing a validate callback, which gets fired any time that state changes, whether it be via state.current = "whatever" or manually modifying the local storage in the browser?

@huntabyte huntabyte changed the title PersistedStore: Add a way to validate values PersistedState: Add a way to validate values Dec 2, 2024
@webJose
Copy link
Contributor Author

webJose commented Dec 2, 2024

Not exactly. Actually, our private version at work has 2 functions: validate and normalize. The former returns a Boolean; the latter receives the (new) value, validates it and makes any necessary changes so the return value is the "final" value.

I don't like it very much. There's some overlapping, right? I designed it like that, but I'm not happy.

Anyway, the lifecycle of the value starts by reading the value. Maybe you're reading it for the first time after upgrading your logic. Now this guy needs to be "upgraded" before its first use. So in my terms:

  • Read from storage.
  • Normalize.
  • Store in reactive variable for general availability.

When the reactive variable changes:

  • Validate it to ensure compliance.
  • If invalid, normalize it.
  • Store.

This is opinionated. If invalid, normalize? Why not reject it as in rollback to the previous value? Things are getting fuzzy. I decided to normalize at work, but I admit it doesn't have any backing other than me saying so.

So there: Something along these lines. I think something like this is definitely needed so one can reliably work with the store directly while its value evolves over time.

@webJose
Copy link
Contributor Author

webJose commented Dec 2, 2024

Oh, but every time you call normalize there's a chance that the value changes, so always store after normalizing.

@huntabyte huntabyte added the help wanted Open to contributions from the community. label Dec 2, 2024
@webJose
Copy link
Contributor Author

webJose commented Dec 14, 2024

I have been juggling around some ideas for this, and this is what I have so far. Let's see if this is acceptable.

Let's start with some type definitions. This is storage.d.ts:

export interface ValueSerializer<T> {
    serialize(value: T): string;
    deserialize(value: string): T;
}

export type PushedValueHandler<T> = (value: T | null) => void;

export interface StorageMedium<T> {
    read(): Promise<T | null>;
    write(value: T | null): Promise<void>;
    start?: () => (() => void) | void;
    onPushValue?: (handler: PushedValueHandler<T>) => (() => void);
}

StorageMedium

The idea here is to define an asynchronous contract for any storage medium people might think of, from the obvious local and session storage, to Redis, Mongo, cookies, indexedDB, SharedArrayBuffer shared with a worker, etc.

The read() and write() methods are obvious, I suppose.

start

This is a method that is meant to be invoked to perform any startup tasks the storage medium requires. Now that I think about it, it probably should be asynchronous. The return value is the cleanup function, if one is needed.

onPushValue

The storage medium may have the ability to receive updates from outside the persisted state object. The perfect example is the storage event for local storage, where a new value may come from a different browser window. This method allows the persisted state object to subscribe and be notified whenever the storage medium has received an update value from "outside".

PersistedState

With this, persisted state options are completely removed, as they are specific to local or session storage, and PersistedState is now agnostic to the particulars of storage. Its constructor becomes:

constructor(initialValue: T, storage: StorageMedium<T>, options: PersistedStateOptions<T> = {})

The options are now of an entirely different nature. Let's see about my proposal for options.

PersistedStateOptions

type PersistedStateOptions<T> = {
	onReadError?: 'ignore' | 'warn' | ((cause: any) => void);
	onWriteError?: 'ignore' | 'warn' | ((cause: any) => void);
	validator?: (value: T) => [boolean, T?];
};

Through options, we allow consumers of the class to specify what should happen if an error is thrown while reading or writing the value to storage. You can probably infer the details by looking at the type.

validator

This one is more interesting. It is a function that is given any new value that comes either via the current property, or via the registered handler with onPushValue in the storage medium. It must return a tuple, where the first value is a Boolean value that indicates if the given value passed all validation tests. This tells PersistedState that write() can be called.

If the value, is false, however, the given value was deemed invalid and must be exchanged with the value in the second position of the tuple. This time, instead of storage.write(), PersistedState sets this.current to the value in the tuple, which will re-trigger this logic. At this point, validator is re-run and the value should be OK now, proceeding with a call to write().

The above can be improved with a control flag to avoid calling validator on the value the validator spat the first time.

Possibilities

I know @huntabyte wants to be able to store a value simultaneously to a cookie. This design currently allows for just one storage medium. We could:

a. Allow the specification of multiple storage media (so say, session storage and a cookie storage).
b. Create a special storage medium that "groups" storage media.

The former has some demons: What storage should have preference when reading or writing the value? Should one storage be used for reading and the other for writing? What happens with onPushValue when having multiple storage media? Etc.

I think the more sensible one would be the latter option. People can then answer the questions above by themselves for their own scenario.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Open to contributions from the community.
Projects
None yet
Development

No branches or pull requests

2 participants