nbubna / store

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

Use a more meaningful test key for localStorage #78

Closed Narigo closed 4 years ago

Narigo commented 4 years ago

Okay, I admint this is probably a pretty esoteric feature request. I was trying to find out what caused this very short blinking in the local storage development tab:

image

I had to take a video from the screen to see that a "_-bad-_": "wolf"was added on each refresh of storybook. Looks like they are using your module internally which in turn creates this key/value for a very short amount of time. Googling "bad", "wolf" and "localStorage" did not yield this result and I'm happy I actually found it through GitHub at all.

It would be nice to change the testKey to something like "__store2_test_key": "ok" or anything that makes the paranoid dev aware of where to look for this and understand that this is not something nefarious going on... ;)

Thank you for your consideration!

nbubna commented 4 years ago

Heh. Yes, given that it's a Doctor Who reference, i'm not surprised Google was unhelpful. I agree, given the rise in usage, something more "professional" and less mysterious is in order. Must admit i'm rather shocked anyone saw this happen in action, rather than just notice it in the source.

Want to do a PR so you can get credit? Totally optional, i'll do it if you don't. Though, i'd personally go with _store2test, as "key" seems redundant. :

Narigo commented 4 years ago

Here you are: https://github.com/nbubna/store/pull/79 I did not check out / try the code as setting up the environment would take me too long right now. I only stumbled across this while debugging and wanted to understand where it came from...

While looking at the code, could store._area = _.storage('fake'); go into the catch block? Or might it be set before the test in some cases?

nbubna commented 4 years ago

All good. And yeah, that could go in the catch block. I probably won't immediately push a new release with your PR change, just because it's functionally no different. But it's merged and will be in the next release. Thanks!