From 4b27f586257924363ca814426da8423298f40b45 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Mon, 22 Mar 2021 14:19:50 +0100 Subject: [PATCH 1/2] Re-factor how the `BasePreferences.prefs`-property is initialized Looking at this now, I cannot understand why we'd need to initialize `this.prefs` with all of the values from `this.defaults`. Not only does this *indirectly* require one extra loop, via the `Object.assign`-call, but it also means that in GENERIC-builds changes to default-preference values might not be picked-up unless the the existing user-prefs are cleared (if the user had *manually* set prefs previously). --- web/preferences.js | 26 ++++++++------------------ 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/web/preferences.js b/web/preferences.js index 125007031..6e8b9b048 100644 --- a/web/preferences.js +++ b/web/preferences.js @@ -35,25 +35,16 @@ class BasePreferences { enumerable: true, configurable: false, }); - this.prefs = Object.assign(Object.create(null), this.defaults); + this.prefs = Object.create(null); this._initializedPromise = this._readFromStorage(this.defaults).then( prefs => { - if (!prefs) { - return; - } - for (const name in prefs) { - const defaultValue = this.defaults[name], - prefValue = prefs[name]; - // Ignore preferences not present in, or whose types don't match, - // the default values. - if ( - defaultValue === undefined || - typeof prefValue !== typeof defaultValue - ) { - continue; + for (const name in this.defaults) { + const prefValue = prefs?.[name]; + // Ignore preferences whose types don't match the default values. + if (typeof prefValue === typeof this.defaults[name]) { + this.prefs[name] = prefValue; } - this.prefs[name] = prefValue; } } ); @@ -86,7 +77,7 @@ class BasePreferences { */ async reset() { await this._initializedPromise; - this.prefs = Object.assign(Object.create(null), this.defaults); + this.prefs = Object.create(null); return this._writeToStorage(this.defaults); } @@ -114,8 +105,7 @@ class BasePreferences { value = value.toString(); } else { throw new Error( - `Set preference: "${value}" is a ${valueType}, ` + - `expected a ${defaultType}.` + `Set preference: "${value}" is a ${valueType}, expected a ${defaultType}.` ); } } else { From be521832115f42c58fc263cb6fb60a41d120bfd0 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Mon, 22 Mar 2021 14:30:03 +0100 Subject: [PATCH 2/2] Remove some *indirect* loops in the `BasePreferences.getAll`-method In the `getAll`-method, we can have just one *explicit* loop rather than two indirect ones via the old `Object.assign`-call. Also, changes the `get`-method to be slightly more compact (while keeping the logic intact). --- web/preferences.js | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/web/preferences.js b/web/preferences.js index 6e8b9b048..ea1ffd483 100644 --- a/web/preferences.js +++ b/web/preferences.js @@ -125,18 +125,13 @@ class BasePreferences { */ async get(name) { await this._initializedPromise; - const defaultValue = this.defaults[name]; + const defaultValue = this.defaults[name], + prefValue = this.prefs[name]; if (defaultValue === undefined) { throw new Error(`Get preference: "${name}" is undefined.`); - } else { - const prefValue = this.prefs[name]; - - if (prefValue !== undefined) { - return prefValue; - } } - return defaultValue; + return prefValue !== undefined ? prefValue : defaultValue; } /** @@ -146,7 +141,13 @@ class BasePreferences { */ async getAll() { await this._initializedPromise; - return Object.assign(Object.create(null), this.defaults, this.prefs); + const obj = Object.create(null); + + for (const name in this.defaults) { + const prefValue = this.prefs[name]; + obj[name] = prefValue !== undefined ? prefValue : this.defaults[name]; + } + return obj; } }