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

Use removeItem in the storage on logout #208

Merged
merged 1 commit into from May 24, 2016

Conversation

Projects
None yet
2 participants
@daffl
Copy link
Member

daffl commented May 18, 2016

removeItem is supported by both, LocalStorage and AsyncStorage so this PR adds it to the dummy storage and uses it when logging out (instead of setting it to an empty string).

Closes #203

@daffl daffl added the in progress label May 18, 2016

@ekryski

This comment has been minimized.

Copy link
Member

ekryski commented May 18, 2016

I'll need to double check RN but I'm pretty sure this is a much better fix than the empty string hack we had to add because AsyncStorage was barfing on null values.

@daffl

This comment has been minimized.

Copy link
Member Author

daffl commented May 24, 2016

@ekryski Got a chance to try it? AsyncStorage does support removeItem so I don't think it would cause any issues with RN.

@ekryski

This comment has been minimized.

Copy link
Member

ekryski commented May 24, 2016

I didn't get a chance to check it. Running into issues getting the chat app to build but I'm pretty sure this is fine. :shipit:

We can revert if it's no good. I'll check it in a couple days.

@ekryski ekryski merged commit c75b20e into master May 24, 2016

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@ekryski ekryski deleted the storage-remove-203 branch May 24, 2016

@ekryski ekryski removed the in progress label May 24, 2016

@daffl

This comment has been minimized.

Copy link
Member Author

daffl commented Jun 9, 2016

This has been released with v0.7.8. I think it should work but let's monitor to see it we get any error report regarding this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.