From 34facdc5c22e87c6c9fe4c01510ba41316dea0c2 Mon Sep 17 00:00:00 2001 From: relrelb Date: Fri, 24 Sep 2021 09:25:03 +0300 Subject: [PATCH] extension: Use `Map` instead of `Record` to store option elements This has few advantages: * `Map` is more performant, and its keys cannot clash with builtin JavaScript properties (e.g. `toString`). * TypeScript has better type information about `map.keys()`, whereas `Object.keys()` always return `string[]`. Also, move `camelize` inside `getBooleanElements`, as it's only used there, and unify the 2 `for` loops in `bindBooleanOptions` (iterating `options` is wrong because it might contain options that doesn't exist in the page). --- web/packages/extension/src/common.ts | 50 +++++++++++++--------------- 1 file changed, 24 insertions(+), 26 deletions(-) diff --git a/web/packages/extension/src/common.ts b/web/packages/extension/src/common.ts index dd755e113..8c575e6d5 100644 --- a/web/packages/extension/src/common.ts +++ b/web/packages/extension/src/common.ts @@ -1,27 +1,26 @@ import * as utils from "./utils"; -function camelize(s: string) { - return s.replace(/[^a-z\d](.)/gi, (_, char) => char.toUpperCase()); -} - export interface Options { ruffleEnable: boolean; ignoreOptout: boolean; } function getBooleanElements() { - const elements: Record< - string, + const camelize = (s: string) => + s.replace(/[^a-z\d](.)/gi, (_, char) => char.toUpperCase()); + + const elements = new Map< + keyof Options, { option: Element; checkbox: HTMLInputElement; label: HTMLLabelElement } - > = {}; + >(); for (const option of document.getElementsByClassName("option")) { const [checkbox] = option.getElementsByTagName("input"); if (checkbox.type !== "checkbox") { continue; } const [label] = option.getElementsByTagName("label"); - const key = camelize(checkbox.id); - elements[key] = { option, checkbox, label }; + const key = camelize(checkbox.id) as keyof Options; + elements.set(key, { option, checkbox, label }); } return elements; } @@ -30,28 +29,26 @@ export async function bindBooleanOptions( onChange?: (options: Options) => void ): Promise { const elements = getBooleanElements(); + const options = await utils.getOptions(Array.from(elements.keys())); - // Bind initial values. - const options = await utils.getOptions(Object.keys(elements)); - for (const [key, value] of Object.entries(options)) { - elements[key].checkbox.checked = value; - } - - for (const [key, { checkbox, label }] of Object.entries(elements)) { - // TODO: click/change/input? - checkbox.addEventListener("click", () => { - const value = checkbox.checked; - options[key as keyof Options] = value; - utils.storage.sync.set({ [key]: value }); - }); - - label.textContent = utils.i18n.getMessage(`settings_${checkbox.id}`); + for (const [key, { checkbox, label }] of elements.entries()) { + // Bind initial value. + checkbox.checked = options[key]; // Prevent transition on load. // Method from https://stackoverflow.com/questions/11131875. label.classList.add("notransition"); label.offsetHeight; // Trigger a reflow, flushing the CSS changes. label.classList.remove("notransition"); + + // TODO: click/change/input? + checkbox.addEventListener("click", () => { + const value = checkbox.checked; + options[key] = value; + utils.storage.sync.set({ [key]: value }); + }); + + label.textContent = utils.i18n.getMessage(`settings_${checkbox.id}`); } // Listen for future changes. @@ -61,10 +58,11 @@ export async function bindBooleanOptions( } for (const [key, option] of Object.entries(changes)) { - if (!elements[key]) { + const element = elements.get(key as keyof Options); + if (!element) { continue; } - elements[key].checkbox.checked = option.newValue; + element.checkbox.checked = option.newValue; options[key as keyof Options] = option.newValue; }