Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

Fix: Added expiration date on local storage #16

Merged
merged 1 commit into from
Nov 7, 2019

Conversation

swatikode
Copy link
Contributor

@swatikode swatikode commented Nov 7, 2019

What is the purpose of this pull request? (Check any that's applicable)

  • Feature
  • Bug fix
  • Dependency updates
  • Documentation updates
  • Build related changes
  • Changes an existing functionality
  • Other, please explain:

Does this introduce a breaking change?

  • Yes
  • No

What changes did you make? (Give an overview)
Releases were retrieved even when tabs were flipped. This caused a lot of screen flickering and unnecessary calls to API to get releases. I added a expiry date of 24 hrs on the local storage item. If the releases are present in the local storage then they will be read. Otherwise the API call will be made - resulting in either 200 or 304 status depending on whether the resource has changed.

Add any screenshots

@abhijit945 abhijit945 added the bug Something isn't working label Nov 7, 2019
@@ -4,6 +4,17 @@ import constants from "./constants";
export const retrieveReleases = gitHubURL =>
new Promise((resolve, reject) => {
const repositoryName = gitHubURL.split("github.com")[1];
const lsReleasesData = localStorage.getItem(constants.LOCAL_STORAGE.RELEASES_DATA) || "";
const releasesData = lsReleasesData.split(";")[0];
Copy link
Contributor

Choose a reason for hiding this comment

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

If the entry is not there then wont this be null? Does this not cause error say in a fresh page?
Did you try this after clearing the localStorage?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes tried it after deleting the local storage manually so that it gets written again. The ||"" on line 7 will set the lsReleasesData to an empty string so there will not be any errors

Copy link
Contributor

@abhijit945 abhijit945 left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍

@abhijit945 abhijit945 merged commit 2e72dbb into master Nov 7, 2019
@abhijit945 abhijit945 deleted the LimitReleasesCalls branch November 7, 2019 22:16
abhijit945 pushed a commit that referenced this pull request Nov 7, 2019
## [1.7.1](v1.7.0...v1.7.1) (2019-11-07)

### Fix

* Added expiration date on local storage (#16) ([2e72dbb](2e72dbb)), closes [#16](#16)
@abhijit945
Copy link
Contributor

🎉 This PR is included in version 1.7.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants