refactor(storage): update storage plugin #154
Conversation
| serialize: JSON.stringify, | ||
| deserialize: JSON.parse | ||
| }; | ||
|
|
||
| export function engineFactory(engine: StorageOption): StorageEngine { | ||
| if (engine === StorageOption.LocalStorage) { | ||
| return localStorage; |
There was a problem hiding this comment.
We are still bound to the global context.
return localStorage
is the same as
return global.localStorage
Where is the handling of the custom storage engine, let's say on mobiles via NativeScript.
| getItem(key): any; | ||
| setItem(key, val): void; | ||
| removeItem(key): void; | ||
| } |
There was a problem hiding this comment.
we can add length, key and clear as well to make it fully compatible.
There was a problem hiding this comment.
Everything else looks great. We just need to test the non-default option, for example sessionStorage.
There was a problem hiding this comment.
Ill make sure it is tested. Just wanted your input before I went to far
docs/plugins/storage.md
Outdated
| key: '@@STATE', | ||
|
|
||
| /** | ||
| * Storage strategy to use. Thie defaults to LocalStorage but you |
| /** | ||
| * Custom deseralizer. Defaults to JSON.parse | ||
| */ | ||
| deserialize: JSON.parse, |
There was a problem hiding this comment.
@un33k - If passing a different storage engine breaks, wouldn't this break too?
There was a problem hiding this comment.
this is actually something I am looking into now. writing tests around a simple custom storage option now
There was a problem hiding this comment.
@amcdnl so it will break depending on what you do with your implementation. it is easy enough to pass a custom serializer/deserialzer though so im not to concerned. maybe a documentation issue more then anything else
docs/plugins/storage.md
Outdated
| /** | ||
| * Custom serializer, defaults to JSON.stringify | ||
| */ | ||
| serialize: JSON.stringify, |
|
@amcdnl don't merge yet. still adding tests and docs around it |
|
@deebloo Looks fantastic. One quick note on the Great job guys. |
|
It would be yes. I can certainly add clarification in the docs |
Updates storage plugin to allow you to pass in your own storage implementation.
@un33k wanted your input since you had opened a PR for this previously. wanted to know what you thought. should address #140