nbubna / store

A better way to use localStorage and sessionStorage
MIT License
1.91k stars 110 forks source link

_.replace does not get rich objects #117

Closed kyeotic closed 1 year ago

kyeotic commented 1 year ago

The docs have this lovely pitch for custom serialization

Likewise, setter functions which take an optional overwrite parameter can also use that parameter to accept a "replacer" function that receives each key and value (yes, nested ones too) as arguments and allow you to provide an alternate means of stringifying the values. This is particularly useful for rich objects like Date types. See MDN's JSON.stringify docs for more information and examples. Alternately, you can set a global replacer to the store._.replace property to handle all set, setAll, add, and transact calls.

But its not true: if you create a function on store._.replace and serialize an object with a Date Key it will receive a Date value that has already been serialized to a string.

Am I missing something, or has this regressed somehow?

Vanilla Sandbox Example

nbubna commented 1 year ago

Hmm. Your sandbox example didn't have code to demonstrate this. But i think i see the problem. It's that we avoid sending functions to JSON.stringify and the _.replace check happens in that expression. I'll try to get a fix up soon.

nbubna commented 1 year ago

Turns out, JSON.stringify appears to be the issue:

Try this: JSON.stringify({ d: new Date() }, (key, value) => value instanceof Date ? "Date!" : value)

Spoiler: it doesn't return '{"d":"Date!"}'. Not sure why...

nbubna commented 1 year ago
image

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify

So it's because Date has a toJSON() method which supersedes the replacer function. Not sure there's any easy way to get the behavior you want, and honestly, i'm not sure i even should do that behavior.

kyeotic commented 1 year ago

Sorry about that, somehow the sandbox got reset to default. Glad you noticed the issue in any case.

I can workaround this problem, but I think the docs should be updated to reflect this. They specifically call out this case and it doesn't work.