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

Extract quota exceeded testing to use an array #88

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Flygenring
Copy link

This improves readability while making it simpler to maintain and extend

This improves readability while making it simpler to maintain and extend
@pamelafox
Copy link
Owner

Hmmm. Maybe this is me being overly curmudgeonly, but I believe that Array.indexOf is a more recent addition to browsers than localStorage, so introducing it could potentially reduce the usability of lscache across browsers. I'm hesitant to make a change that could reduce the browser compatibility, if it doesn't have a big benefit. Anyone else have thoughts?

@peter-gribanov
Copy link
Contributor

About browsers support localStorage and Array.indexOf.
The main problem as always in IE.

According to StatCounter:

  • IE8 uses 0.38%
  • IE9 uses 0.25%.

According to NetMarketShare:

  • IE6 uses 0.18%
  • IE7 uses 0.16%
  • IE8 uses 1.06%
  • IE9 uses 0.46%

In my opinion, this optimization does not add readability and performance.
I would recommend to postpone this PR for some time.

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.

3 participants