Skip to content
This repository has been archived by the owner on Sep 10, 2019. It is now read-only.

Localstorage only removes single item, instead of clearing entire cache #30

Merged
merged 1 commit into from
Nov 30, 2015
Merged

Localstorage only removes single item, instead of clearing entire cache #30

merged 1 commit into from
Nov 30, 2015

Conversation

erikfried
Copy link
Contributor

Solves #29

Please, this is a very serious bug, so it would be great if you could get this released soon and deprecate any other 2.x releases.

The consequence of this bug was that as soon as session is refreshed using the local storage module, the entire localstorage was wiped.

@erikfried
Copy link
Contributor Author

ping @joawan @thogra

@thogra
Copy link
Contributor

thogra commented Nov 26, 2015

👍

@@ -25,7 +25,7 @@ function set(key, value, expiresInSeconds) {

function clear(key) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should maybe look into renaming this function name as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe, I wasn´t bold enough to make any changes more than needed.
I think the name might be there because it was called 'clear' in the cookie module.

If changed it think it should be changed there as well

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm thinking that renaming the method is probably a thing for a separate PR (as that's more an issue of style), if we decide that it's the right thing to do.

@Andersos
Copy link
Contributor

Thanks for finding this and submitting a pull request.

Andersos added a commit that referenced this pull request Nov 30, 2015
Localstorage only removes single item, instead of clearing entire cache
@Andersos Andersos merged commit da2fb23 into schibsted:master Nov 30, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants