Allow persist middleware to accept 'localStorage' and 'sessionStorage' for easier usage of session

[RobbyUitbeijerse]

Hi there @AnatoleLucet and @dai-shi !

After the middleware localStorage PR was closed I quickly ran into the situation where we actually wanted to persist state in session. I can imagine though that most developers wouldn't actually want this as the store persist in memory most of the time - but in our specific case we have multiple front-ends hosted on the same domain and the user would switch between them on multiple occasions in the same session, where we actually wanted to get back the stored data when they would come back to our frontend where the Zustand store was initially created.

Of course it's possible to pass window.sessionStorage, but you are left doing the same check that initially had to be done for localStorage in order to make it work in SSR. Allowing the user to pass 'localStorage' and 'sessionStorage' as string options and retrieving the apis from the window if it's defined solves it - but not sure if this is a welcome change since @dai-shi actually preferred not checking window.

Let me know what you think

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 5cc14e9:

Sandbox	Source
React	Configuration
React Typescript	Configuration

[dai-shi]

Yeah, I'd prefer to make storage null or false, to allow this.

persist(..., { name: 'foo', storage: typeof sessionStorage !== 'undefined' ? sessionStorage : null })

But, if it's rather common to specify a string like storage: 'localStorage', it's acceptable.

In either case, the documentation could be tricky.

[AnatoleLucet]

@dai-shi I'm not quite sure to understand why you don't want to check for the window object in the middleware.
If the user still wants to bypass that window check, we could add an option for it.
But maybe I'm missing something?

[dai-shi]

Just checking window existence is not enough because some envs like test might not have localStorage even if it mocks window. (This can be improved lately, but I think checking localStorage existence is more straightforward and safe.)

However, this PR resolves this concern, because it checks both. So, I'm fine with it. (Unless there is any env that has localStorage without window, in the future.)
My hesitation is more about how much this middleware should support. If someone requests, would we add 'indexeddb'? I'm a bit concerned about the string configs.

That said, this PR is acceptable to me, because we already support localStorage from the beginning. (I would have proposed the default to be no storage, but it'd not be very handy for localStorage users.)

[AnatoleLucet]

Ah, didn't though about test env.
It's true that giving a string feels a bit weird.

[RobbyUitbeijerse]

If it feels weird I'm not sure if it's the right approach, alternatively I could make an effort on changing the documentation to be a bit more helpful on the SSR situation for users that would prefer using sessionStorage over localStorage.

Alternatively, we could export a SSR safe instance of both session as well as localStorage from the middleware, allowing users to do something like;

import { sessionStorage, persist } from 'zustand/middleware'

export const useStore = create(persist(
  (set, get) => ({
   ...
  }),
  {
    name: "key",
    storage: sessionStorage,
  }
))

but then again, not sure if that's worth the trouble. A README update might just as well be enough.

[AnatoleLucet]

That'd be "safer" than a plain string (e.g. typing mistakes), but we'd need to provide pretty much any kind of dummy store that the user could want to use.

[dai-shi]

(I still think it's an option to pass strings, if this is the final one.)

I didn't know SSR users prefer/require sessionStorage.
It's a bit regrettable that we chose localStorage as default.

How are you (un)comfortable with my first suggestion?
If you pass null, the persist middleware uses dummy internally (or just skip).

[AnatoleLucet]

How are you (un)comfortable with my first suggestion?
If you pass null, the persist middleware uses dummy internally (or just skip).

Sorry, totally forgot this com. It could be a nice solution.

[dai-shi]

#248 (comment)

@RobbyUitbeijerse Would it be not too comfortable? Should we go with strings?

Ping @ianstormtaylor for another nextjs user.

[ianstormtaylor]

FWIW, I think it would be nice to make the 90% cases for all of these not require the user having to write checks environment checks for the options. I think doing something simple like offering multiple persistence functions instead like hooks tend to do would be a nice solution.

Similar to how react-use exposes:

import { useLocalStorage, useSessionStorage } from 'react-use'

I think it would make sense to expose:

import { create, persistLocalStorage, persistSessionStorage } from 'zustand'

This would avoid the problem entirely. And a more generic persist can still be exposed for complex cases where people want to do custom things for storage, but the 90% cases are nicely covered.

---

Also FWIW, I think the current pattern of:

import create from 'zustand'
import { persist } from 'zustand/middleware'

Is an anti-pattern. It makes it hard to remember what the "proper" way to import something from the library is. And with bundlers supporting tree-shaking these days it isn't required anymore to make bundles smaller. And it can often mess up Intellisense-style autocomplete because the exports aren't as easily automatically found.

[dai-shi]

persist, persistLocalStorage, persistSessionStorage

I prefer this pattern than strings. This would be a breaking change, though.

(edit) But, it's not very cute, compared to other ones.

---

(Slightly off topic)

And with bundlers supporting tree-shaking these days it isn't required anymore to make bundles smaller.

I'm aware of this. But, it's not only about tree-shaking and app bundle size. We want to explicitly separate it from core.

I think the alternative would be:

import create from 'zustand'
import { persist } from 'zustand-middleware' // or '@zustand/middleware

and make it monorepo. But, maybe it's not the trend either.

And it can often mess up Intellisense-style autocomplete

I wasn't aware of this. Would adding "exports" in package.json help by chance? Don't know.

[ianstormtaylor]

@dai-shi nice, yeah in terms of separation of core, that makes sense.

I think the monorepo is the way to go if you want to actually think and develop them as separate packages. And then you could even call it zustand-persist instead, and have the middleware import not be a grab bag. (And then it wouldn't be a breaking change right now either, new package entirely.)

But in addition, maybe "what is in core" is a concept that needs to be rethought. I'd argue that…

import create from 'zustand'

…is kind of a "cute" attempt at simplicity, but isn't really the "truth". (I say this as someone who often falls for the cute!) For example, is zustand/shallow part of core? It seems like a fairly "core" kind of need and feels weird to have a separate package for it. What about zustand/vanilla? Seems to me like the it's only called "vanilla" because the React-specific one wasn't named "react", if non-framework is really a goal.

I think as soon as you change the import to…

import { create } from 'zustand'

…you'll be able to separate the valid semantic question of "what do we want to have as part of this library's core?" from the cuteness of having a single default export.

[dai-shi]

I personally like named imports but the cuteness would be important for this library.

Good point on zustand/vanilla. There are many libs with framework-agnostic core and react binding, like redux and react-redux. I guess our primary target is React and no-framework is not really a goal.

---

Intellisense-style autocomplete is still a concern to me. Let me investigate how vscode would work.

OK, this is off topic from the original PR. Let me make a new issue later.

[dai-shi]

So, what should we do with this.

1. leave it for more discussion
2. merge as is
3. counter proposal to accept null instead
4. another proposal with persistLocalStorage and persisteSessionStorage

[ianstormtaylor]

FWIW, another thought I had...

I originally wanted Local Storage, but realized soon after that I wanted to have it defaulted from the URL as well (ie. in the case of something like TypeScript Playground where the URL includes the state as well as remembering your last snippet).

The existing API for persist is very much designed around Local/Session Storage, but I'm not sure that's the least opinionated way is to do it. I might suggest keeping it even simpler and allowing for passing in just two functions: get and set. Something like:

persist(..., {
  get: () => JSON.parse(localStorage.getItem('my-key')),
  set: value => localStorage.setItem('my-key', JSON.stringify(value)),
})

This would make it infinitely flexible, and very clear for people exactly how they can augment the behavior... just write whatever they want. For example, you could imagine layering in location.replace(...) in the case of wanting to update the URL. Or if you want to change the serialization logic, it's simple to think about.

You can still then have the more purpose-built helpers with sane defaults which most people will want to use:

persistLocalStorage('my-key', ...)

But that way the lowest level utility is actually as customizable as you need it to be. And as a side benefit, your purpose-built helpers can be more opinionated, like assuming JSON serialization, to keep their own APIs simpler.

I say all this because I wanted to use Local Storage, but soon realized I needed something slightly different, and if you don't expose the lower level you'll be locking out these kinds of use cases.

---

And, back to our off-topic convo... when thinking about "what's in core", the decision becomes easier:

• zustand — persist
• zustand-local-storage — persistLocalStorage
• zustand-session-storage — persistSessionStorage

[dai-shi]

My perspective is that persist doesn't need to cover 100% use cases. Just 60% would be ok.
To me, it's more like a recipe, anyone can grab the code and create their own version. So, defaulting to localStorage made sense. And, my suggestion is option 3, because I personally don't want to make it too complicated which is less like a recipe.

All that said, I would like to leave @AnatoleLucet for suggestion.

[ianstormtaylor]

@dai-shi you get the final call, but personally, I don't think that kind of thinking leads to good library design.

It will just end up frustrating users who hit the artificial limit you've imposed when they have to change their approach from importing to copy-pasting just to barely tweak some logic. I mean look at the code for persist… no one will understand that at a glance, so that's just straight up debt that they're forced to copy-paste into their codebase. And if they're using persist in some places and the copy-pasted helper in others, now you've doubled the LOC in their bundle size.

My take would be… If you're truly looking to make a "recipe" then put it in a Gist, or in the docs, so that it's always copy-pasted. If instead you're looking to add a useful utility to the core of a library that aims to be small in size, make it flexible.

[dai-shi]

Yeah, I'd admit the current version is already too complicated.

[AnatoleLucet]

Well, I don't personally have a strong opinion on all that, but:
I think we should try to support as most use cases as this middleware can, BUT, there's always going to be a limit to that. Because yes, persist is currently mainly suited for simple / common usages. So, does that mean we need to rethink its design? (probably)

I don't think what @ianstormtaylor proposed here is a good idea for the persist middleware.

persist(..., {
  get: () => JSON.parse(localStorage.getItem('my-key')),
  set: value => localStorage.setItem('my-key', JSON.stringify(value)),
})

persist should handle all that for you by default (set, get, serialization, deserialization, etc...). But it could definitely be an interesting middleware, just not suited for a ready-to-use persist middleware.
I think we should look at and try to be inspired by what other largely used persisting state utilities have done, because after all they've probably gone through the same path and discussions as we currently are.

---

For your discussion about the design of Zustand's core, I think this is something that should involve more people in a separated issue.

[ianstormtaylor]

Totally agree! It should not be called "persist" if it's that low-level/flexible—it's more of a building block. It could be called effect or something similar.

[dai-shi]

I think we should try to support as most use cases as this middleware can, BUT, there's always going to be a limit to that.

@AnatoleLucet Alright. I think this makes sense.
Unfortunately I don't have much experience in this "persist" domain, and I was a bit too conservative.
So, I would like to ask someone to volunteer maintaining this "persist" middleware dedicatedly.
You came first in my mind as the original author. Will you be able to take the lead?

[AnatoleLucet]

@dai-shi surely, why not. I mean, I'm neither some kind of "persistent storage savvy" and that's why I said it'd be nice to check what others have made which fits the community's needs.
But ok, I can take the lead if you'd like me to.

[dai-shi]

@AnatoleLucet That's great to hear. Would you join the pmndrs discord and DM me there please?

[dai-shi]

@AnatoleLucet is now a collaborator on the zustand repo and will take care of persist. Welcome! 👍

[AnatoleLucet]

What about:

persist(..., {
  name: "foo",
  storage: () => sessionStorage,
})

Then we can just call options.storage() in a try catch. If an error is caught it means the sessionStorage is unavailable.
This is the cleanest way I can think about. Any thoughts?

[dai-shi]

Yeah, it seems clean. Do you keep the previous behavior or make a breaking change? Either is fine from my perspective.

[AnatoleLucet]

Since the middleware has been introduced recently, an easy to fix breaking change like this one should be ok

[sexta13]

With this breaking change, is there any way to define a dynamic key? I would like to have something like this: [user_id]_key.
Right now I'm doing this using a customHook to get the authenticated user id, and then building session storage accordingly...
Will be something like that be possible?

[AnatoleLucet]

@sexta13 not sure to fully understand. Do you mean a custom key which point to a storage (e.g. window.[storageKey])? Maybe you can send an example?

[sexta13]

Sure. I guess I didn't make myself very clear (after reading it better).

What I want/need is that the name used in the options for sessionStorage can be dynamic.

The use case is: I have multiple users that use the same browser/window and they cannot share the same sessionStorage, so in order to identify which sessionStorage a user should use I have something like this: [userId]_myapp.
How can I accomplish that with zustand and persist with sessionStorage?

If this is not clear enough, tell me that I'll try to post some code ;)

[AnatoleLucet]

I don't think this change will break your code. See, the storage given to the persist middleware is just an api which ables the middleware to interact with some kind of storage (FYI, it needs to fit localStorage/sessionStorage's api, so for example AsyncStorage is good).
The middleware will simply use the name when interacting with the storage (e.g. storage.getItem("87_key"), storage.setItem("87_key", newState)).

[sexta13]

Well, I guess it's me don't knowing how to do it :)
Looking to this snippet:

import create from "zustand"
import { persist } from "zustand/middleware"

export const useStore = create(persist(
  (set, get) => ({
    fish: 0,
    addAFish: () => set({ fish: get().fish + 1 })
  }),
  {
    name: "food-storage", // unique name
    storage: sessionStorage, // (optional) default is 'localStorage'
  }
))

How can I pass a name to this, so that the name can be something built from outside? Or then inside this method have something to call a function that returns a name? Or even use a hook that has that info?

Can it be something like this?

import` create from "zustand"
import { persist } from "zustand/middleware"

export const useStore =(storeName)=> create(persist(
  (set, get) => ({
    fish: 0,
    addAFish: () => set({ fish: get().fish + 1 })
  }),
  {
    name: storeName, // unique name
    storage: sessionStorage, // (optional) default is 'localStorage'
  }
))

and then call it likeuseFoodStore('my_name_123')()? it looks odd....

[AnatoleLucet]

This should work, but only if you use this store in one place... There's probably another way to do this, but as you said, it looks odd.
We should probably add some kind of api to interact with the middleware. I'll open an issue for it.