add setStorageType from localStorageService

[william57m]

A lot of users and me need to change storage type according to application behaviors, so I add setStorageType method directly callable from localStorageService. Maybe there are better solution (for example, give a storageType parameter directly in set method...), but I think this can help people.

[coveralls]

Coverage decreased (-0.81%) when pulling 4335557 on william57m:master into 0f28dad on grevory:master.

[grevory]

Thanks!

You are able to change the storage type in the configuration. Can you explain your reasoning for this? There is code duplication here.

There would also need to a be a test for this.

[coveralls]

Coverage increased (+0.17%) when pulling 607200f on william57m:master into 0f28dad on grevory:master.

[william57m]

Yes we can change the storage type in configuration, but it's fixed in whole application, unless if we create a second factory based on the provider (something like this : #151 (comment)) but we must inject two services into our controller and doing with this, it's not convenient.
I'm working to fix failed tests.

[coveralls]

Coverage increased (+0.22%) when pulling d31f17f on william57m:master into 0f28dad on grevory:master.

[coveralls]

Coverage increased (+2.41%) when pulling ce274fc on william57m:master into 0f28dad on grevory:master.

[william57m]

I also add the support of storageType parameters in all functions, this way we can make:

localStorageService.set('foo', 'bar', 'sessionStorage');
localStorageService.set('foo', 'bar', 'localStorage');
localStorageService.remove('foo', 'sessionStorage');
localStorageService.get('foo', 'sessionStorage'); // -> null
localStorageService.get('foo', 'localStorage'); // -> 'bar'
localStorageService.get('foo'); // -> 'bar' (default value of storageType)
localStorageService.length('sessionStorage'); -> 0
localStorageService.keys('localStorage'); -> ['foo']

By default functions take default storageType parameters defined with setStorageType method.

[a8m]

I can't agree with that, it's break SoC, it's conflicting the input and output of the service and you need to maintain this logic(storage switching) in your application.
I think we should fix #151, and then we be able to deal with this two storage separately.

[william57m]

Ok I understand, it was a suggestion. But it could be interesting to manage web storage like cookies (with expiration date), local storage and session storage would be mixed and the user should not need to set the storage type, he shouldn't need to know how it is managed. This way, the user could give a parameter as 'keep' for example to keep the storage indefinitely in the storage (local storage), and 'not keep' to store data temporarily (in session storage). And the get method will search key in local and session.
But it maybe not the goal of this project.

[a8m]

First, I really appreciate your contributing... so, thanks.
But like I said, I think we should support both types(local and session),
then you be able to create your own factory that wraps both storage and simply implement the suggest behaviour with the keep option, etc..

[ealves-pt]

@william57m I know it has been a full more that 1yr since you have created this PR. Can I ask you to rebase with the master and try to fix the conflicts? I would really like to merge this functionality.

If you don't want to waste time I understand and I can figure out a way to merge this.

[william57m]

Hi @ealves-pt I'll take a look this week if I can do it quickly.

Re: actually it's not possible to resolve conflicts after rebase, there has been too many updates. The better is to start again from master.

[ealves-pt]

@william57m yeah you are right, I have looked into your code and there a couple of things that have changed. Any help is welcome 👍

Thanks

[grevory]

#351 covers this. Thanks guys!