Core refactor (#1207)

* Copy set intersection functions

* Remove unused functions

* Simplify url check

* Remove parseUrl

* Simplify stringReverse

* Remove hasOwn due to infrequent use

* Rename errorToJson/jsonToError to de/serializeError

For clarity on intended use.

* Fix time argument on timeout

* Add missing return value

* Throw an error for unexpected argument values

* Add documentation comments

.eslintrc.json

@@ -99,16 +99,11 @@
                 "ext/mixed/js/dictionary-data-util.js"
             ],
             "globals": {
-                "errorToJson": "readonly",
+                "serializeError": "readonly",
-                "jsonToError": "readonly",
+                "deserializeError": "readonly",
                 "isObject": "readonly",
-                "hasOwn": "readonly",
                 "stringReverse": "readonly",
                 "promiseTimeout": "readonly",
-                "parseUrl": "readonly",
-                "areSetsEqual": "readonly",
-                "getSetIntersection": "readonly",
-                "getSetDifference": "readonly",
                 "escapeRegExp": "readonly",
                 "deferPromise": "readonly",
                 "clone": "readonly",

ext/bg/js/audio-downloader.js

@@ -189,7 +189,7 @@ class AudioDownloader {
             throw new Error('No custom URL defined');
         }
         const data = {expression, reading};
-        const url = customSourceUrl.replace(/\{([^}]*)\}/g, (m0, m1) => (hasOwn(data, m1) ? `${data[m1]}` : m0));
+        const url = customSourceUrl.replace(/\{([^}]*)\}/g, (m0, m1) => (Object.prototype.hasOwnProperty.call(data, m1) ? `${data[m1]}` : m0));
         return {type: 'url', details: {url}};
     }
 

ext/bg/js/backend.js

@@ -305,7 +305,7 @@ class Backend {
             try {
                 this._validatePrivilegedMessageSender(sender);
             } catch (error) {
-                callback({error: errorToJson(error)});
+                callback({error: serializeError(error)});
                 return false;
             }
         }
@@ -628,7 +628,7 @@ class Backend {
     }
 
     _onApiLog({error, level, context}) {
-        yomichan.log(jsonToError(error), level, context);
+        yomichan.log(deserializeError(error), level, context);
     }
 
     _onApiLogIndicatorClear() {
@@ -667,7 +667,7 @@ class Backend {
                 const result = this._modifySetting(target);
                 results.push({result: clone(result)});
             } catch (e) {
-                results.push({error: errorToJson(e)});
+                results.push({error: serializeError(e)});
             }
         }
         await this._saveOptions(source);
@@ -681,7 +681,7 @@ class Backend {
                 const result = this._getSetting(target);
                 results.push({result: clone(result)});
             } catch (e) {
-                results.push({error: errorToJson(e)});
+                results.push({error: serializeError(e)});
             }
         }
         return results;
@@ -730,8 +730,10 @@ class Backend {
 
         const isTabMatch = (url2) => {
             if (url2 === null || !url2.startsWith(baseUrl)) { return false; }
-            const {baseUrl: baseUrl2, queryParams: queryParams2} = parseUrl(url2);
+            const parsedUrl = new URL(url2);
-            return baseUrl2 === baseUrl && (queryParams2.mode === mode || (!queryParams2.mode && mode === 'existingOrNewTab'));
+            const baseUrl2 = `${parsedUrl.origin}${parsedUrl.pathname}`;
+            const mode2 = parsedUrl.searchParams.get('mode');
+            return baseUrl2 === baseUrl && (mode2 === mode || (!mode2 && mode === 'existingOrNewTab'));
         };
 
         const openInTab = async () => {
@@ -1052,7 +1054,7 @@ class Backend {
         const cleanup = (error) => {
             if (port === null) { return; }
             if (error !== null) {
-                port.postMessage({type: 'error', data: errorToJson(error)});
+                port.postMessage({type: 'error', data: serializeError(error)});
             }
             if (!hasStarted) {
                 port.onMessage.removeListener(onMessage);

ext/bg/js/options.js

@@ -343,7 +343,7 @@ class OptionsUtil {
 
         const combine = (target, source) => {
             for (const key in source) {
-                if (!hasOwn(target, key)) {
+                if (!Object.prototype.hasOwnProperty.call(target, key)) {
                     target[key] = source[key];
                 }
             }

ext/bg/js/settings/dictionary-import-controller.js

@@ -338,7 +338,7 @@ class DictionaryImportController {
         const errors = [];
         for (const {error} of results) {
             if (typeof error !== 'undefined') {
-                errors.push(jsonToError(error));
+                errors.push(deserializeError(error));
             }
         }
         return errors;

ext/bg/js/settings/generic-setting-controller.js

@@ -113,7 +113,7 @@ class GenericSettingController {
     _transformResults(values, targets) {
         return values.map((value, i) => {
             const error = value.error;
-            if (error) { return jsonToError(error); }
+            if (error) { return deserializeError(error); }
             const {metadata: {transforms}, element} = targets[i];
             const result = this._applyTransforms(value.result, transforms, 'post', element);
             return {result};

ext/bg/js/template-renderer-proxy.js

@@ -127,7 +127,7 @@ class TemplateRendererProxy {
                 cleanup();
                 const {error} = response;
                 if (error) {
-                    reject(jsonToError(error));
+                    reject(deserializeError(error));
                 } else {
                     resolve(response.result);
                 }

ext/bg/js/translator.js

@@ -980,6 +980,30 @@ class Translator {
         }
     }
 
+    _areSetsEqual(set1, set2) {
+        if (set1.size !== set2.size) {
+            return false;
+        }
+
+        for (const value of set1) {
+            if (!set2.has(value)) {
+                return false;
+            }
+        }
+
+        return true;
+    }
+
+    _getSetIntersection(set1, set2) {
+        const result = [];
+        for (const value of set1) {
+            if (set2.has(value)) {
+                result.push(value);
+            }
+        }
+        return result;
+    }
+
     // Reduction functions
 
     _getTermTagsScoreSum(termTags) {
@@ -1181,11 +1205,11 @@ class Translator {
 
     _createMergedGlossaryTermDefinition(source, rawSource, definitions, expressions, readings, allExpressions, allReadings) {
         const only = [];
-        if (!areSetsEqual(expressions, allExpressions)) {
+        if (!this._areSetsEqual(expressions, allExpressions)) {
-            only.push(...getSetIntersection(expressions, allExpressions));
+            only.push(...this._getSetIntersection(expressions, allExpressions));
         }
-        if (!areSetsEqual(readings, allReadings)) {
+        if (!this._areSetsEqual(readings, allReadings)) {
-            only.push(...getSetIntersection(readings, allReadings));
+            only.push(...this._getSetIntersection(readings, allReadings));
         }
 
         const sourceTermExactMatchCount = this._getSourceTermMatchCountSum(definitions);

ext/mixed/js/api.js

@@ -40,7 +40,7 @@ const api = (() => {
 
             yomichan.on('log', async ({error, level, context}) => {
                 try {
-                    await this.log(errorToJson(error), level, context);
+                    await this.log(serializeError(error), level, context);
                 } catch (e) {
                     // NOP
                 }
@@ -264,7 +264,7 @@ const api = (() => {
                             break;
                         case 'error':
                             cleanup();
-                            reject(jsonToError(message.data));
+                            reject(deserializeError(message.data));
                             break;
                     }
                 };
@@ -317,7 +317,7 @@ const api = (() => {
                         this._checkLastError(chrome.runtime.lastError);
                         if (response !== null && typeof response === 'object') {
                             if (typeof response.error !== 'undefined') {
-                                reject(jsonToError(response.error));
+                                reject(deserializeError(response.error));
                             } else {
                                 resolve(response.result);
                             }

ext/mixed/js/comm.js

@@ -150,7 +150,7 @@ class CrossFrameAPIPort extends EventDispatcher {
 
         const error = data.error;
         if (typeof error !== 'undefined') {
-            invocation.reject(jsonToError(error));
+            invocation.reject(deserializeError(error));
         } else {
             invocation.resolve(data.result);
         }
@@ -200,7 +200,7 @@ class CrossFrameAPIPort extends EventDispatcher {
     }
 
     _sendError(id, error) {
-        this._sendResponse({type: 'result', id, data: {error: errorToJson(error)}});
+        this._sendResponse({type: 'result', id, data: {error: serializeError(error)}});
     }
 }
 

ext/mixed/js/core.js

@@ -15,13 +15,14 @@
  * along with this program.  If not, see <https://www.gnu.org/licenses/>.
  */
 
-/*
+/**
- * Error handling
+ * Converts an `Error` object to a serializable JSON object.
+ * @param error An error object to convert.
+ * @returns A simple object which can be serialized by `JSON.stringify()`.
  */
-
+function serializeError(error) {
-function errorToJson(error) {
     try {
-        if (isObject(error)) {
+        if (typeof error === 'object' && error !== null) {
             return {
                 name: error.name,
                 message: error.message,
@@ -38,77 +39,56 @@ function errorToJson(error) {
     };
 }
 
-function jsonToError(jsonError) {
+/**
-    if (jsonError.hasValue) {
+ * Converts a serialized erorr into a standard `Error` object.
-        return jsonError.value;
+ * @param serializedError A simple object which was initially generated by serializeError.
+ * @returns A new `Error` instance.
+ */
+function deserializeError(serializedError) {
+    if (serializedError.hasValue) {
+        return serializedError.value;
     }
-    const error = new Error(jsonError.message);
+    const error = new Error(serializedError.message);
-    error.name = jsonError.name;
+    error.name = serializedError.name;
-    error.stack = jsonError.stack;
+    error.stack = serializedError.stack;
-    error.data = jsonError.data;
+    error.data = serializedError.data;
     return error;
 }
 
-
+/**
-/*
+ * Checks whether a given value is a non-array object.
- * Common helpers
+ * @param value The value to check.
+ * @returns `true` if the value is an object and not an array, `false` otherwise.
  */
-
 function isObject(value) {
     return typeof value === 'object' && value !== null && !Array.isArray(value);
 }
 
-function hasOwn(object, property) {
+/**
-    return Object.prototype.hasOwnProperty.call(object, property);
+ * Converts any string into a form that can be passed into the RegExp constructor.
-}
+ * https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular_Expressions
-
+ * @param string The string to convert to a valid regular expression.
-// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular_Expressions
+ * @returns The escaped string.
+ */
 function escapeRegExp(string) {
     return string.replace(/[.*+\-?^${}()|[\]\\]/g, '\\$&');
 }
 
+/**
+ * Reverses a string.
+ * @param string The string to reverse.
+ * @returns The returned string, which retains proper UTF-16 surrogate pair order.
+ */
 function stringReverse(string) {
-    return string.split('').reverse().join('').replace(/([\uDC00-\uDFFF])([\uD800-\uDBFF])/g, '$2$1');
+    return [...string].reverse().join('');
-}
-
-function parseUrl(url) {
-    const parsedUrl = new URL(url);
-    const baseUrl = `${parsedUrl.origin}${parsedUrl.pathname}`;
-    const queryParams = Array.from(parsedUrl.searchParams.entries())
-        .reduce((a, [k, v]) => Object.assign({}, a, {[k]: v}), {});
-    return {baseUrl, queryParams};
-}
-
-function areSetsEqual(set1, set2) {
-    if (set1.size !== set2.size) {
-        return false;
-    }
-
-    for (const value of set1) {
-        if (!set2.has(value)) {
-            return false;
-        }
-    }
-
-    return true;
-}
-
-function getSetIntersection(set1, set2) {
-    const result = [];
-    for (const value of set1) {
-        if (set2.has(value)) {
-            result.push(value);
-        }
-    }
-    return result;
-}
-
-function getSetDifference(set1, set2) {
-    return new Set(
-        [...set1].filter((value) => !set2.has(value))
-    );
 }
 
+/**
+ * Creates a deep clone of an object or value. This is similar to `JSON.parse(JSON.stringify(value))`.
+ * @param value The value to clone.
+ * @returns A new clone of the value.
+ * @throws An error if the value is circular and cannot be cloned.
+ */
 const clone = (() => {
     // eslint-disable-next-line no-shadow
     function clone(value) {
@@ -176,6 +156,12 @@ const clone = (() => {
     return clone;
 })();
 
+/**
+ * Checks if an object or value is deeply equal to another object or value.
+ * @param value1 The first value to check.
+ * @param value2 The second value to check.
+ * @returns `true` if the values are the same object, or deeply equal without cycles. `false` otherwise.
+ */
 const deepEqual = (() => {
     // eslint-disable-next-line no-shadow
     function deepEqual(value1, value2) {
@@ -242,6 +228,11 @@ const deepEqual = (() => {
     return deepEqual;
 })();
 
+/**
+ * Creates a new base-16 (lower case) string of a sequence of random bytes of the given length.
+ * @param length The number of bytes the string represents. The returned string's length will be twice as long.
+ * @returns A string of random characters.
+ */
 function generateId(length) {
     const array = new Uint8Array(length);
     crypto.getRandomValues(array);
@@ -252,11 +243,10 @@ function generateId(length) {
     return id;
 }
 
-
+/**
-/*
+ * Creates an unresolved promise that can be resolved later, outside the promise's executor function.
- * Async utilities
+ * @returns An object `{promise, resolve, reject}`, containing the promise and the resolve/reject functions.
  */
-
 function deferPromise() {
     let resolve;
     let reject;
@@ -267,6 +257,12 @@ function deferPromise() {
     return {promise, resolve, reject};
 }
 
+/**
+ * Creates a promise that is resolved after a set delay.
+ * @param delay How many milliseconds until the promise should be resolved. If 0, the promise is immediately resolved.
+ * @param resolveValue Optional; the value returned when the promise is resolved.
+ * @returns A promise with two additional properties: `resolve` and `reject`, which can be used to complete the promise early.
+ */
 function promiseTimeout(delay, resolveValue) {
     if (delay <= 0) {
         const promise = Promise.resolve(resolveValue);
@@ -303,6 +299,13 @@ function promiseTimeout(delay, resolveValue) {
     return promise;
 }
 
+/**
+ * Creates a promise that will resolve after the next animation frame, using `requestAnimationFrame`.
+ * @param timeout Optional; a maximum duration (in milliseconds) to wait until the promise resolves. If null or omitted, no timeout is used.
+ * @returns A promise that is resolved with `{time, timeout}`, where `time` is the timestamp from `requestAnimationFrame`,
+ *  and `timeout` is a boolean indicating whether the cause was a timeout or not.
+ * @throws The promise throws an error if animation is not supported in this context, such as in a service worker.
+ */
 function promiseAnimationFrame(timeout=null) {
     return new Promise((resolve, reject) => {
         if (typeof cancelAnimationFrame !== 'function' || typeof requestAnimationFrame !== 'function') {
@@ -327,7 +330,7 @@ function promiseAnimationFrame(timeout=null) {
                 cancelAnimationFrame(frameRequest);
                 frameRequest = null;
             }
-            resolve({time: timeout, timeout: true});
+            resolve({time: performance.now(), timeout: true});
         };
 
         // eslint-disable-next-line no-undef
@@ -338,16 +341,23 @@ function promiseAnimationFrame(timeout=null) {
     });
 }
 
-
+/**
-/*
+ * Base class controls basic event dispatching.
- * Common classes
  */
-
 class EventDispatcher {
+    /**
+     * Creates a new instance.
+     */
     constructor() {
         this._eventMap = new Map();
     }
 
+    /**
+     * Triggers an event with the given name and specified argument.
+     * @param eventName The string representing the event's name.
+     * @param details Optional; the argument passed to the callback functions.
+     * @returns `true` if any callbacks were registered, `false` otherwise.
+     */
     trigger(eventName, details) {
         const callbacks = this._eventMap.get(eventName);
         if (typeof callbacks === 'undefined') { return false; }
@@ -355,8 +365,14 @@ class EventDispatcher {
         for (const callback of callbacks) {
             callback(details);
         }
+        return true;
     }
 
+    /**
+     * Adds a single event listener to a specific event.
+     * @param eventName The string representing the event's name.
+     * @param callback The event listener callback to add.
+     */
     on(eventName, callback) {
         let callbacks = this._eventMap.get(eventName);
         if (typeof callbacks === 'undefined') {
@@ -366,6 +382,12 @@ class EventDispatcher {
         callbacks.push(callback);
     }
 
+    /**
+     * Removes a single event listener from a specific event.
+     * @param eventName The string representing the event's name.
+     * @param callback The event listener callback to add.
+     * @returns `true` if the callback was removed, `false` otherwise.
+     */
     off(eventName, callback) {
         const callbacks = this._eventMap.get(eventName);
         if (typeof callbacks === 'undefined') { return true; }
@@ -383,44 +405,85 @@ class EventDispatcher {
         return false;
     }
 
+    /**
+     * Checks if an event has any listeners.
+     * @param eventName The string representing the event's name.
+     * @returns `true` if the event has listeners, `false` otherwise.
+     */
     hasListeners(eventName) {
         const callbacks = this._eventMap.get(eventName);
         return (typeof callbacks !== 'undefined' && callbacks.length > 0);
     }
 }
 
+/**
+ * Class which stores event listeners added to various objects, making it easy to remove them in bulk.
+ */
 class EventListenerCollection {
+    /**
+     * Creates a new instance.
+     */
     constructor() {
         this._eventListeners = [];
     }
 
+    /**
+     * Returns the number of event listeners that are currently in the object.
+     * @returns The number of event listeners that are currently in the object.
+     */
     get size() {
         return this._eventListeners.length;
     }
 
+    /**
+     * Adds an event listener of a generic type.
+     * @param type The type of event listener, which can be 'addEventListener', 'addListener', or 'on'.
+     * @param object The object to add the event listener to.
+     * @param args The argument array passed to the object's event listener adding function.
+     * @throws An error if type is not an expected value.
+     */
     addGeneric(type, object, ...args) {
         switch (type) {
             case 'addEventListener': return this.addEventListener(object, ...args);
             case 'addListener': return this.addListener(object, ...args);
             case 'on': return this.on(object, ...args);
+            default: throw new Error(`Invalid type: ${type}`);
         }
     }
 
+    /**
+     * Adds an event listener using `object.addEventListener`. The listener will later be removed using `object.removeEventListener`.
+     * @param object The object to add the event listener to.
+     * @param args The argument array passed to the `addEventListener`/`removeEventListener` functions.
+     */
     addEventListener(object, ...args) {
         object.addEventListener(...args);
         this._eventListeners.push(['removeEventListener', object, ...args]);
     }
 
+    /**
+     * Adds an event listener using `object.addListener`. The listener will later be removed using `object.removeListener`.
+     * @param object The object to add the event listener to.
+     * @param args The argument array passed to the `addListener`/`removeListener` function.
+     */
     addListener(object, ...args) {
         object.addListener(...args);
         this._eventListeners.push(['removeListener', object, ...args]);
     }
 
+    /**
+     * Adds an event listener using `object.on`. The listener will later be removed using `object.off`.
+     * @param object The object to add the event listener to.
+     * @param args The argument array passed to the `on`/`off` function.
+     */
     on(object, ...args) {
         object.on(...args);
         this._eventListeners.push(['off', object, ...args]);
     }
 
+    /**
+     * Removes all event listeners added to objects for this instance and clears the internal list of event listeners.
+     */
     removeAllEventListeners() {
         if (this._eventListeners.length === 0) { return; }
         for (const [removeFunctionName, object, ...args] of this._eventListeners) {

ext/mixed/js/yomichan.js

@@ -219,7 +219,7 @@ const yomichan = (() => {
             }
             error = response.error;
             if (error) {
-                throw jsonToError(error);
+                throw deserializeError(error);
             }
             return response.result;
         }
@@ -233,7 +233,7 @@ const yomichan = (() => {
                 if (async) {
                     promiseOrResult.then(
                         (result) => { callback({result}); },
-                        (error) => { callback({error: errorToJson(error)}); }
+                        (error) => { callback({error: serializeError(error)}); }
                     );
                     return true;
                 } else {
@@ -241,7 +241,7 @@ const yomichan = (() => {
                     return false;
                 }
             } catch (error) {
-                callback({error: errorToJson(error)});
+                callback({error: serializeError(error)});
                 return false;
             }
         }