Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

switch/select type dynamically #320

Merged
merged 9 commits into from
Jul 29, 2016
Merged

switch/select type dynamically #320

merged 9 commits into from
Jul 29, 2016

Conversation

zpgu
Copy link
Contributor

@zpgu zpgu commented Jul 13, 2016

  1. added setStorageType(type)
  2. added type to relevant calls except removeFromLocalStorage(...), as it's a vararg variety, but easy to do if needed.

This is a non-intrusive patch, as it preserves original functionality with minimal changes, but adds storage type selectability when needed.

Typical use case:
your existing code picked one type as default, but in a couple of places you need to use the other type, then try this:

setStorageType(non-default-type); // switch to new type
...do your storage calls
setStorageType(default-type); // restore default

Or you can specify a type to each call directly (without type specified, will use whatever the last type active).

@grevory
Copy link
Owner

grevory commented Jul 13, 2016

I like the changes here but it decreases test coverage. Could you write some tests to cover the changes you made? Thanks!

1) added setStorageType(type)
2) added type to relevant calls except removeFromLocalStorage(...), as it's a vararg variety, but easy to do if needed.

This is a non-intrusive patch, as it preserves original functionality with minimal changes, but adds storage type selectability when needed.

Typical use case:
your existing code picked one type as default, but in a couple of places you need to use the other type, then try this:

setStorageType(non-default-type); // switch to new type
...do your storage calls
setStorageType(default-type); // restore default

Or you can specify a type to each call directly (without type specified, will use whatever the last type active).
zpgu and others added 6 commits July 13, 2016 22:10
removeFromLocalStorage() supports selecting storage type as the last parameter.

not as optimized as I'd like for the patches, but they're non-invasive, and easy to reason for correctness.
This completes the type selection patch (TODO: need to add tests).
removeFromLocalStorage() to take type param
@grevory grevory merged commit 482c6c1 into grevory:master Jul 29, 2016
@grevory
Copy link
Owner

grevory commented Jul 29, 2016

Thanks.

@JR-Utily
Copy link

This should be documented in the readme to let people know it is possible.
I was about to make a workaround myself !

@benhoIIand
Copy link
Contributor

benhoIIand commented Feb 2, 2017

Not sure this was the best implementation - why, when specified in the call directly localStorageService.set('status', true, 'sessionStorage'); does it actually switch the default storage? Why does it not use the specified type of storage just for that transaction? Was it implemented this way for a reason?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants