Update everything to more modern React!

[ryanjyost]

I've been lazy folks....

[scraggo]

I'd like to help out. What specifically do you want to update? I had src/index open in my text editor and went through some of the errors that airbnb's react eslint reported.

[ryanjyost]

Nice! Adding linting rules would be great.

But I wanted to convert everything to use hooks, functional components, etc. but the current implementation requires that the parent component be a Class component, so this library won't work with functional components.

If you could figure out a way to somehow use context/hooks/etc. to sync with localStorage, that'd be awesome!

There are already useLocalStorage hooks out there, so I don't want to just copy those solutions, but that might just be the best solution 🤷‍♂️

[scraggo]

Context could be promising. I personally like how this is implemented and don't feel a burning need to convert it to use hooks (especially if hook versions exist elsewhere).

While I've got you here, the reason I was digging into this library was to add a "pre-save" functionality. I noticed that an ES6 Set always stringifies to an empty object {}. I was able to implement a way to do convert data from the calling end. I was wondering if you'd be interested in seeing this - I could file an issue and put in a PR if so.

[ryanjyost]

Sounds great @scraggo, go for it!

And I'm worried that the current way this works will become unusable once everyone has adopted modern React paradigms, which avoid Class components

[scraggo]

First, apologies for not really understanding what you were saying earlier.

this library won't work with functional components.

I hadn't thought about this when I was using your library in a few stateful components in my project.

I've been messing with using your existing API to work with a functional component that uses hooks. So far it's working and I'm hoping to post something for you to review soon.

[scraggo]

Here it is: https://github.com/scraggo/local-storage-hooks/

Here's where I've edited the source of SimpleStorage: https://github.com/scraggo/local-storage-hooks/blob/master/src/external/SimpleStorage.js

Here are the two example components: https://github.com/scraggo/local-storage-hooks/tree/master/src/components

(The git history was squashed and is now unhelpful to look at, sorry about that!)

[ryanjyost]

I'll take a look @scraggo, is this something you'd want to open a PR for into this library

[scraggo]

Definitely would open a PR. I made this sandbox app to make it easier for me to try out the changes. Curious to see if you like how the hooks API works.

[ryanjyost]

I never would have thought of the Hook implementation, very cool. Curious though if/how it's better than just using a hook like https://usehooks.com/useLocalStorage/?

EDIT:
I guess it'd be easier to have a built-in reset functionality.

[scraggo]

re: the link you sent - the idea of the hook syncing with setValue is pretty neat. It's different from how SimpleStorage works - which syncs on pagehide. (Correct me if I'm wrong).

I think it's your call if you think my proposal is better. It definitely has more boilerplate in that the user has to re-declare their hooks in the SimpleStorage component.

[ryanjyost]

I think we could just provide both, why not, right?

So if you were to open a PR with what you showed me, I think that'd be a great addition to the library

[ryanjyost]

I'll add the useLocalStorage type hook in a separate PR.