Closed barrymun closed 10 months ago
@chris-mosley there are some nice changes in this pr; i've tried to document all of the alterations but please have a look through the description.
the background script has been given a big rework so that we only set the storage values once to avoid race conditions where some (or all) of the values may not actually exist in storage. i've also improved the typing around the getStorageValue
and setStorageValue
functions so it should tell us if we're trying to set values in storage that should not be set (for example, we should not set brandsMap
in sync
storage as this will result in errors for chrome extensions).
i've also included the basic work for a test suite which can be expanded on to make the code more resilient in the future too.
i would appreciate it if you could test this one before merge when you get a chance, as there are a lot of changes. if there are any issues, or you spot any problems when using the extension/addon, please let me know and i can try to fix before we merge. if you have any other questions please don't hesitate to ask. i've tested as best i can and everything seems to work correctly on my end, but it's always good to get another pair of eyes on larger prs. thanks
Did a fair amount of testing on this and it seems to be working. I did uninstall and reload it a few times but i believe when i do that I get a new guid for the addon, so I'm not certain that it really tested overwriting of old settings or not.
Description
content
andpopup
scripts, as thebackground
script did not set the values before they were readlocal
andsync
storage) to avoid confusion and to guarantee one version of settings is set in the background script, and that retry logic exists in thepopup
andcontent
scripts so that the settings are always retrieved (unless there is a problem fetching the brands list (this should be addressed in another pr maybe))Changes
setStorageValue
function now uses function overloading to prevent certain values being set in sync storagelodash
lib and corresponding typessetTimeout
changed tosetInterval
so that the brand list update check function is run every day, not just once a day latersync
settings, only local settings. it is impossible to set "brandsMap" in sync settings in chrome anyway because of sync storage size restrictionsjest
quickly)jest
). the commandyarn test
will run the testsgetSettings
function which will first check for sync settings but fall back to local if sync settings are not available for whatever reasongetStorageValue
andsetStorageValue
functionssetStorageValue
function, where ifoverride
is set it will clear all values in storage before setting the new value.normal
is the default settingprop
param tostorageArea
for improved readabilityStorage<>
types for improved readability