search

IITC-CE / ingress-intel-total-conversion

intel.ingress.com total conversion user script with some new features. Should allow easier extension of the intel map.
https://iitc.app
ISC License
285 stars 110 forks source link

Update `localStorage` use? #737

Open nexushoratio opened 3 months ago

nexushoratio commented 3 months ago

While working on a personal plugin, I noticed this bit from MDN's Using the Web Storage API:

Note: It's recommended to use the Web Storage API (setItem, getItem, removeItem, key, length) to prevent the pitfalls associated with using plain objects as key-value stores.

So I switched my code over to it. Out of curiosity, I checked the main code base:

$ find -type f -exec grep 'localStorage\.' {} + | wc -l
19
$ find -type f -exec grep 'localStorage\[' {} + | wc -l
88

(only one came from external source)

Would it make sense update all direct property usage of localStorage to using the methods, just for best practices? Then maybe update whatever linter configs to catch that (if possible) ?

I know, a small thing, but since I just did it myself, I thought I would ask.

Also, I wonder if it would make sense to offer any IITC helper functions? In particular, anything that might detect quota issues and pop up an alert, rather than each bit of code (that currently does NOT do that), having to do so themselves.

I did see an issue about possible alternatives to using localStorage directly (#230). It seems like nothing moved forward on it, and I wonder if it is still an issue with modern browsers. With my plugin, I also managed to max out the quota, but I didn't notice any slow down related to large rewrites. But, I don't use mobile, so not sure if it is problematic there, and such a library migration might still make sense (over my thought).