feat(middleware/persist): improve createJSONStorage for Maps

[lauhon]

Related Issues or Discussions

Fixes #618

Summary

The related Issue is closed because a workaround exists. But I would love to see that possible without a workaround needed. Thats why I did this fix.

Check List

• yarn run prettier for formatting code and docs
• Add tests to check persistance and rehydration in sync and async mode (I just extended existing tests instead of writing new ones, I hope thats ok)
• implement reviver and replacer for JSON.stringify & JSON.parse

[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 c9c7a98:

Sandbox	Source
React	Configuration
React Typescript	Configuration
React Browserify	Configuration
React Snowpack	Configuration
React Parcel	Configuration
@pavlobu/zustand demo	Configuration

[dai-shi]

Hi, thanks for suggestion.
Our design decision with #1463 is to let users specify any storage, so that we don't need to support everything. (ex. #1749 (comment) )

So, the general suggestion is to provide an alternative function to createJOSNStorage in third-party packages. We basically don't accept changes to the default createJSONStorage.

That said, what would be acceptable is to let createJSONStorage have a second argument options and allow specifying reviver and replacer. #1720 kind of suggests it.

Would you like to work on it? You would still like to create a third-party package that supports Maps out of the box.

[lauhon]

Hey, thanks for the answer!

Alright, makes sense to me.

Just to double check if I understand correctly:

What I would do now is to

1. extend createJsonStorage by an optional parameter Options which for now can contain a replacer and a reviver function with the signatures of the code I already have in this PR.

2. The replacing/reviving code itself should not live in zustand, so what I would do is publish a new Lib that contains replacer & reviver (I found no existing npm package that contains that for Map/Set)

3. (OPTIONAL) pull the new lib as dev-dependency into zustand and use it with the new options parameter in the tests here. Then I can basically keep the tests I already did, maybe then it makes even more sense to create a new test just for the new options object..

Please let me know if those steps make sense for you, and if you want me to even do the third step. If not there would then be the question if and how I should test the options parameter 🤔

[dai-shi]

@lauhon
1: Yes. We'll see details during the review process.
2: You have two options for your package. One is to provide replacer/reviver and the other is to provide createJSONStorageWithMaps to be used instead of createJSONStorage.
3: No, please don't pull a new dev dependency. Just define replacer/reviver in the test code, which is the code you already have in the lib code.

[dai-shi]

@lauhon Hi, just wondering how you are doing. No rush, but hope to have the update soon.

[lauhon]

Hey @dai-shi, thanks for the push. Was a couple of busy days for me :)

Not sure if its in your interest what i did in the tests. Looking forward to your review!

[dai-shi]

Overall looks good to me. Added some comments.

[dai-shi]

I think we can make these optional:

Suggested change

	reviver: (key: string, value: unknown) => unknown
	replacer: (key: string, value: unknown) => unknown
	reviver?: (key: string, value: unknown) => unknown
	replacer?: (key: string, value: unknown) => unknown

[lauhon]

Done

[dai-shi]

Could you create a new test case for Map and keep existing tests unmodified?

[lauhon]

Done, i pretty much copied the old persist- and rehyadrate tests and changed it to persist and rehydrate a map, I hope thats fine

[dai-shi]

Please check lint errors.

[lauhon]

Done

[dai-shi]

Thanks for your contribution!

[Jackardios]

I think in the same way we could serialize / deserialize Sets

function replacer(key: string, value: any) {
  if (value instanceof Map) {
    return { __type: "Map", value: Object.fromEntries(value) };
  }
  if (value instanceof Set) {
    return { __type: "Set", value: Array.from(value) };
  }
  return value;
}

function reviver(key: string, value: any) {
  if (value?.__type === "Map") {
    return new Map(Object.entries(value.value));
  }
  if (value?.__type === "Set") {
    return new Set(value.value);
  }
  return value;
}

export function createEnhancedJSONStorage<S>(
  getStorage: () => StateStorage
): PersistStorage<S> | undefined {
  let storage: StateStorage | undefined;

  try {
    storage = getStorage();
  } catch (e) {
    // prevent error if the storage is not defined (e.g. when server side rendering a page)
    return;
  }

  const persistStorage: PersistStorage<S> = {
    getItem: (name) => {
      const parse = (str: string | null) => {
        if (str === null) {
          return null;
        }
        return JSON.parse(str, reviver) as StorageValue<S>;
      };
      const str = (storage as StateStorage).getItem(name) ?? null;
      if (str instanceof Promise) {
        return str.then(parse);
      }
      return parse(str);
    },
    setItem: (name, newValue) =>
      (storage as StateStorage).setItem(
        name,
        JSON.stringify(newValue, replacer)
      ),
    removeItem: (name) => (storage as StateStorage).removeItem(name),
  };

  return persistStorage;
}

[dai-shi]

Yes, and my point is that supporting things are out of the scope of Zustand library.