search

cryptonomex / graphene-ui

Cryptonomex Graphene front end (wallet and decentralized exchange)
https://bitshares.openledger.info
MIT License
156 stars 100 forks source link

Status of localStorage abstraction? #824

Closed rulatir closed 8 years ago

rulatir commented 8 years ago

What is the plan and current status regarding the abstraction of local storage?

Currently some stores use dl/src/common/localStorage.js but others don't. AccountStore uses it inconsistently - it still contains direct calls to localStorage.

An abstraction layer around localStorage would be a very welcome foundation for porting to React Native. A simple implementation based on realm database seems to work just fine until code that uses localStorage directly is hit.

I hope dl/src/common/localStorage.js is not an abandoned direction?

svk31 commented 8 years ago

You're right, the abstraction should be used everywhere. If you'd like to update the code to do so I'll happily take PRs for that.

svk31 commented 8 years ago

Nevermind I had a look and updated the localStorage method and changed to using it where appropriate, let me know if that fixes your issue.

rulatir commented 8 years ago

I'll be preparing a PR anyway as I am not very happy with the current solution. You wrap localStorage to provide a simplified API for stores (pre-bound key prefix etc.), but you don't really abstract localStorage implementation. Instead there are these if(ls) checks that essentially deceive application code: "yes, everything is OK, we have localStorage, it's just that nothing has ever been put in there and everything you put in there will not be there either, tee-hee" ;)

I am going to factor out the part that provides the ls object into a separate module localStorageImpl.js and use RN's platform-specific module capability (localStorageImpl.android.js) to provide a realm-based polyfill for android (and later ios).

svk31 commented 8 years ago

Sure, my wrapper was just a quick fix added when I noticed that the javascript cli_wallet (which probably doesn't even work anymore) didn't have access to localStorage.