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

4 files changed, 141 insertions, 78 deletions

diff --git a/ext/mixed/js/api.js b/ext/mixed/js/api.js
index 83b5c083..d7cf4ea2 100644
--- a/ext/mixed/js/api.js
+++ b/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);
                             }
diff --git a/ext/mixed/js/comm.js b/ext/mixed/js/comm.js
index b1958679..4a6786c3 100644
--- a/ext/mixed/js/comm.js
+++ b/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)}});
     }
 }
 
diff --git a/ext/mixed/js/core.js b/ext/mixed/js/core.js
index c34421c7..9305739a 100644
--- a/ext/mixed/js/core.js
+++ b/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 errorToJson(error) {
+function serializeError(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) {
-        return jsonError.value;
-    }
-    const error = new Error(jsonError.message);
-    error.name = jsonError.name;
-    error.stack = jsonError.stack;
-    error.data = jsonError.data;
+/**
+ * Converts a serialized erorr into a standard `Error` object.
+ * @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(serializedError.message);
+    error.name = serializedError.name;
+    error.stack = serializedError.stack;
+    error.data = serializedError.data;
     return error;
 }
 
-
-/*
- * Common helpers
+/**
+ * Checks whether a given value is a non-array object.
+ * @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);
-}
-
-// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular_Expressions
+/**
+ * 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.
+ * @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');
-}
-
-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))
-    );
+    return [...string].reverse().join('');
 }
 
+/**
+ * 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;
 }
 
-
-/*
- * Async utilities
+/**
+ * Creates an unresolved promise that can be resolved later, outside the promise's executor function.
+ * @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) {
     });
 }
 
-
-/*
- * Common classes
+/**
+ * Base class controls basic event dispatching.
  */
-
 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) {
diff --git a/ext/mixed/js/yomichan.js b/ext/mixed/js/yomichan.js
index 32408413..61301e30 100644
--- a/ext/mixed/js/yomichan.js
+++ b/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;
             }
         }