From b20622b2c84ce3ca1781c7bf8e10fed0af1e5001 Mon Sep 17 00:00:00 2001 From: toasted-nutbread Date: Thu, 7 Jan 2021 21:36:20 -0500 Subject: [PATCH] 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 | 9 +- ext/bg/js/audio-downloader.js | 2 +- ext/bg/js/backend.js | 16 +- ext/bg/js/options.js | 2 +- .../settings/dictionary-import-controller.js | 2 +- .../js/settings/generic-setting-controller.js | 2 +- ext/bg/js/template-renderer-proxy.js | 2 +- ext/bg/js/translator.js | 32 ++- ext/mixed/js/api.js | 6 +- ext/mixed/js/comm.js | 4 +- ext/mixed/js/core.js | 201 ++++++++++++------ ext/mixed/js/yomichan.js | 6 +- 12 files changed, 184 insertions(+), 100 deletions(-) diff --git a/.eslintrc.json b/.eslintrc.json index f9ca5814..94551803 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -99,16 +99,11 @@ "ext/mixed/js/dictionary-data-util.js" ], "globals": { - "errorToJson": "readonly", - "jsonToError": "readonly", + "serializeError": "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", diff --git a/ext/bg/js/audio-downloader.js b/ext/bg/js/audio-downloader.js index 9b34be56..19e9c368 100644 --- a/ext/bg/js/audio-downloader.js +++ b/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}}; } diff --git a/ext/bg/js/backend.js b/ext/bg/js/backend.js index 690f6a3c..db83d0b7 100644 --- a/ext/bg/js/backend.js +++ b/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); - return baseUrl2 === baseUrl && (queryParams2.mode === mode || (!queryParams2.mode && mode === 'existingOrNewTab')); + const parsedUrl = new URL(url2); + 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); diff --git a/ext/bg/js/options.js b/ext/bg/js/options.js index 16168e38..adf3cfd4 100644 --- a/ext/bg/js/options.js +++ b/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]; } } diff --git a/ext/bg/js/settings/dictionary-import-controller.js b/ext/bg/js/settings/dictionary-import-controller.js index f392e5a9..436a056c 100644 --- a/ext/bg/js/settings/dictionary-import-controller.js +++ b/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; diff --git a/ext/bg/js/settings/generic-setting-controller.js b/ext/bg/js/settings/generic-setting-controller.js index 95be975e..0d24c429 100644 --- a/ext/bg/js/settings/generic-setting-controller.js +++ b/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}; diff --git a/ext/bg/js/template-renderer-proxy.js b/ext/bg/js/template-renderer-proxy.js index 507d8067..04b53ce7 100644 --- a/ext/bg/js/template-renderer-proxy.js +++ b/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); } diff --git a/ext/bg/js/translator.js b/ext/bg/js/translator.js index 8cc520a8..7242ec56 100644 --- a/ext/bg/js/translator.js +++ b/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)) { - only.push(...getSetIntersection(expressions, allExpressions)); + if (!this._areSetsEqual(expressions, allExpressions)) { + only.push(...this._getSetIntersection(expressions, allExpressions)); } - if (!areSetsEqual(readings, allReadings)) { - only.push(...getSetIntersection(readings, allReadings)); + if (!this._areSetsEqual(readings, allReadings)) { + only.push(...this._getSetIntersection(readings, allReadings)); } const sourceTermExactMatchCount = this._getSourceTermMatchCountSum(definitions); 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 . */ -/* - * 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; +/** + * 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(jsonError.message); - error.name = jsonError.name; - error.stack = jsonError.stack; - error.data = jsonError.data; + 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; } }