Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Breaking out un-related changes from #1487 #1670

Merged
merged 10 commits into from
Dec 16, 2024
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion .eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -396,9 +396,12 @@
"@typescript-eslint/no-unsafe-return": "off",
"@typescript-eslint/require-await": "off",
"@typescript-eslint/restrict-template-expressions": "off",
"@typescript-eslint/prefer-promise-reject-errors": "off",

"@typescript-eslint/ban-ts-comment": ["error", {"ts-expect-error": {"descriptionFormat": "^ - .+$"}}],
"@typescript-eslint/ban-types": ["error", {"types": {"object": true}, "extendDefaults": true}],
"@typescript-eslint/no-empty-object-type": "error",
"@typescript-eslint/no-unsafe-function-type": "error",
"@typescript-eslint/no-wrapper-object-types": "error",
"@typescript-eslint/no-explicit-any": "error",
"@typescript-eslint/no-shadow": ["error", {"builtinGlobals": false}],
"@typescript-eslint/no-this-alias": "error",
Expand Down
5 changes: 4 additions & 1 deletion .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
"eslint.format.enable": true,
"javascript.format.insertSpaceAfterOpeningAndBeforeClosingNonemptyBraces": false,
"typescript.format.insertSpaceAfterOpeningAndBeforeClosingNonemptyBraces": false,
"javascript.format.enable": false,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like this shouldn't be here? I like having files formatted

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The files are still formatted, just by eslint instead of vscode's formatter. Previously they would both run and conflict and cause all sorts of issues.

"typescript.format.enable": false,
"javascript.preferences.importModuleSpecifierEnding": "js",
"editor.tabSize": 4,
"editor.insertSpaces": true,
Expand All @@ -39,5 +41,6 @@
"url": "/ext/data/schemas/recommended-dictionaries-schema.json"
}
],
"typescript.tsdk": "node_modules\\typescript\\lib"
"typescript.tsdk": "node_modules/typescript/lib",
"nixEnvSelector.nixFile": "${workspaceRoot}/shell.nix"
}
5 changes: 3 additions & 2 deletions benches/jsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
"noImplicitAny": true,
"strictPropertyInitialization": true,
"suppressImplicitAnyIndexErrors": false,
"skipLibCheck": false,
"skipLibCheck": true,
"baseUrl": ".",
"paths": {
"*": ["../types/ext/*"],
Expand Down Expand Up @@ -41,6 +41,7 @@
],
"exclude": [
"../node_modules",
"../dev/lib"
"../dev/lib",
"../ext/lib"
]
}
3 changes: 2 additions & 1 deletion dev/jsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,10 @@
"noImplicitAny": true,
"strictPropertyInitialization": true,
"suppressImplicitAnyIndexErrors": false,
"skipLibCheck": false,
"skipLibCheck": true,
"baseUrl": ".",
"paths": {
"api-map": ["../types/ext/api-map"],
"anki-templates": ["../types/ext/anki-templates"],
"anki-templates-internal": ["../types/ext/anki-templates-internal"],
"cache-map": ["../types/ext/cache-map"],
Expand Down
3 changes: 3 additions & 0 deletions ext/js/app/frontend.js
Original file line number Diff line number Diff line change
Expand Up @@ -952,10 +952,13 @@ export class Frontend {
* @returns {Promise<boolean>}
*/
async _scanSelectedText(allowEmptyRange, disallowExpandSelection, showEmpty = false) {
performance.mark('frontend:scanSelectedText:start');
const range = this._getFirstSelectionRange(allowEmptyRange);
if (range === null) { return false; }
const source = disallowExpandSelection ? TextSourceRange.createLazy(range) : TextSourceRange.create(range);
await this._textScanner.search(source, {focus: true, restoreSelection: true}, showEmpty);
performance.mark('frontend:scanSelectedText:end');
performance.measure('frontend:scanSelectedText', 'frontend:scanSelectedText:start', 'frontend:scanSelectedText:end');
return true;
}

Expand Down
2 changes: 1 addition & 1 deletion ext/js/app/popup-factory.js
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ export class PopupFactory {
promises.push(promise);
}

/** @type {undefined|unknown} */
/** @type {undefined|Error} */
let error = void 0;
/** @type {{popup: import('popup').PopupAny, token: string}[]} */
const results = [];
Expand Down
1 change: 1 addition & 0 deletions ext/js/app/popup.js
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,7 @@ export class Popup extends EventDispatcher {
await this._show(sourceRects, writingMode);

if (displayDetails !== null) {
performance.mark('invokeDisplaySetContent:start');
void this._invokeSafe('displaySetContent', {details: displayDetails});
}
}
Expand Down
1 change: 0 additions & 1 deletion ext/js/background/backend.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <https://www.gnu.org/licenses/>.
*/

import {AccessibilityController} from '../accessibility/accessibility-controller.js';
import {AnkiConnect} from '../comm/anki-connect.js';
import {ClipboardMonitor} from '../comm/clipboard-monitor.js';
Expand Down
2 changes: 2 additions & 0 deletions ext/js/comm/anki-connect.js
Original file line number Diff line number Diff line change
Expand Up @@ -479,7 +479,9 @@ export class AnkiConnect {
if (typeof result === 'object' && result !== null && !Array.isArray(result)) {
const apiError = /** @type {import('core').SerializableObject} */ (result).error;
if (typeof apiError !== 'undefined') {
// eslint-disable-next-line @typescript-eslint/no-base-to-string
const error = new ExtensionError(`Anki error: ${apiError}`);
// eslint-disable-next-line @typescript-eslint/no-base-to-string
error.data = {action, params, status: response.status, apiError: typeof apiError === 'string' ? apiError : `${apiError}`};
throw error;
}
Expand Down
4 changes: 2 additions & 2 deletions ext/js/comm/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -390,10 +390,10 @@ export class API {
if (response !== null && typeof response === 'object') {
const {error} = /** @type {import('core').UnknownObject} */ (response);
if (typeof error !== 'undefined') {
reject(ExtensionError.deserialize(/** @type {import('core').SerializedError} */ (error)));
reject(ExtensionError.deserialize(/** @type {import('core').SerializedError} */(error)));
} else {
const {result} = /** @type {import('core').UnknownObject} */ (response);
resolve(/** @type {import('api').ApiReturn<TAction>} */ (result));
resolve(/** @type {import('api').ApiReturn<TAction>} */(result));
}
} else {
const message = response === null ? 'Unexpected null response. You may need to refresh the page.' : `Unexpected response of type ${typeof response}. You may need to refresh the page.`;
Expand Down
2 changes: 1 addition & 1 deletion ext/js/comm/cross-frame-api.js
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ export class CrossFrameAPIPort extends EventDispatcher {
return;
}
}

performance.mark(`cross-frame-api:invoke:${action}`);
try {
this._port.postMessage(/** @type {import('cross-frame-api').InvokeMessage} */ ({type: 'invoke', id, data: {action, params}}));
} catch (e) {
Expand Down
26 changes: 21 additions & 5 deletions ext/js/comm/frame-endpoint.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
*/

import {EventListenerCollection} from '../core/event-listener-collection.js';
import {log} from '../core/log.js';
import {generateId} from '../core/utilities.js';

export class FrameEndpoint {
Expand Down Expand Up @@ -69,19 +70,34 @@ export class FrameEndpoint {
if (this._token !== null) { return; } // Already initialized

const {data} = event;
if (typeof data !== 'object' || data === null) { return; } // Invalid message
if (typeof data !== 'object' || data === null) {
log.error('Invalid message');
return;
}

const {action} = /** @type {import('core').SerializableObject} */ (data);
if (action !== 'frameEndpointConnect') { return; } // Invalid message
if (action !== 'frameEndpointConnect') {
log.error('Invalid action');
return;
}

const {params} = /** @type {import('core').SerializableObject} */ (data);
if (typeof params !== 'object' || params === null) { return; } // Invalid data
if (typeof params !== 'object' || params === null) {
log.error('Invalid data');
return;
}

const {secret} = /** @type {import('core').SerializableObject} */ (params);
if (secret !== this._secret) { return; } // Invalid authentication
if (secret !== this._secret) {
log.error('Invalid authentication');
return;
}

const {token, hostFrameId} = /** @type {import('core').SerializableObject} */ (params);
if (typeof token !== 'string' || typeof hostFrameId !== 'number') { return; } // Invalid target
if (typeof token !== 'string' || typeof hostFrameId !== 'number') {
log.error('Invalid target');
return;
}

this._token = token;

Expand Down
6 changes: 4 additions & 2 deletions ext/js/data/database.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export class Database {
/**
* @param {string} databaseName
* @param {number} version
* @param {import('database').StructureDefinition<TObjectStoreName>[]} structure
* @param {import('database').StructureDefinition<TObjectStoreName>[]?} structure
*/
async open(databaseName, version, structure) {
if (this._db !== null) {
Expand All @@ -43,7 +43,9 @@ export class Database {
try {
this._isOpening = true;
this._db = await this._open(databaseName, version, (db, transaction, oldVersion) => {
this._upgrade(db, transaction, oldVersion, structure);
if (structure !== null) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably be part of the main PR

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems like a guard clause added for type-safety and not really related to the main PR

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the PR makes this field nullable so that we can have workers that interact with the DB but don't upgrade it. It's a logic change.

this._upgrade(db, transaction, oldVersion, structure);
}
});
} finally {
this._isOpening = false;
Expand Down
3 changes: 3 additions & 0 deletions ext/js/dictionary/dictionary-database.js
Original file line number Diff line number Diff line change
Expand Up @@ -455,13 +455,16 @@ export class DictionaryDatabase {
* @returns {Promise<TResult[]>}
*/
_findMultiBulk(objectStoreName, indexNames, items, createQuery, predicate, createResult) {
performance.mark('findMultiBulk:start');
return new Promise((resolve, reject) => {
const itemCount = items.length;
const indexCount = indexNames.length;
/** @type {TResult[]} */
const results = [];
if (itemCount === 0 || indexCount === 0) {
resolve(results);
performance.mark('findMultiBulk:end');
performance.measure('findMultiBulk', 'findMultiBulk:start', 'findMultiBulk:end');
return;
}

Expand Down
11 changes: 8 additions & 3 deletions ext/js/display/display-generator.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,17 +111,23 @@ export class DisplayGenerator {
node.dataset.groupedFrequencyCount = `${groupedFrequencies.length}`;
node.dataset.primaryMatchTypes = [...primaryMatchTypes].join(' ');

performance.mark('displayGenerator:createTermEntry:createTermHeadword:start');
for (let i = 0, ii = headwords.length; i < ii; ++i) {
const node2 = this._createTermHeadword(headwords[i], i, pronunciations);
node2.dataset.index = `${i}`;
headwordsContainer.appendChild(node2);
}
headwordsContainer.dataset.count = `${headwords.length}`;
performance.mark('displayGenerator:createTermEntry:createTermHeadword:end');
performance.measure('displayGenerator:createTermEntry:createTermHeadword', 'displayGenerator:createTermEntry:createTermHeadword:start', 'displayGenerator:createTermEntry:createTermHeadword:end');

performance.mark('displayGenerator:createTermEntry:promises:start');
this._appendMultiple(inflectionRuleChainsContainer, this._createInflectionRuleChain.bind(this), inflectionRuleChainCandidates);
this._appendMultiple(frequencyGroupListContainer, this._createFrequencyGroup.bind(this), groupedFrequencies, false);
this._appendMultiple(groupedPronunciationsContainer, this._createGroupedPronunciation.bind(this), groupedPronunciations);
this._appendMultiple(headwordTagsContainer, this._createTermTag.bind(this), termTags, headwords.length);
performance.mark('displayGenerator:createTermEntry:promises:end');
performance.measure('displayGenerator:createTermEntry:promises', 'displayGenerator:createTermEntry:promises:start', 'displayGenerator:createTermEntry:promises:end');

for (const term of uniqueTerms) {
headwordTagsContainer.appendChild(this._createSearchTag(term));
Expand Down Expand Up @@ -486,7 +492,6 @@ export class DisplayGenerator {
this._appendMultiple(tagListContainer, this._createTag.bind(this), [...tags, dictionaryTag]);
this._appendMultiple(onlyListContainer, this._createTermDisambiguation.bind(this), disambiguations);
this._appendMultiple(entriesContainer, this._createTermDefinitionEntry.bind(this), entries, dictionary);

return node;
}

Expand Down Expand Up @@ -980,7 +985,7 @@ export class DisplayGenerator {
_appendKanjiLinks(container, text) {
let part = '';
for (const c of text) {
if (isCodePointKanji(/** @type {number} */ (c.codePointAt(0)))) {
if (isCodePointKanji(/** @type {number} */(c.codePointAt(0)))) {
if (part.length > 0) {
container.appendChild(document.createTextNode(part));
part = '';
Expand Down Expand Up @@ -1011,7 +1016,7 @@ export class DisplayGenerator {
const {ELEMENT_NODE} = Node;
if (Array.isArray(detailsArray)) {
for (const details of detailsArray) {
const item = createItem(details, /** @type {TExtraArg} */ (arg));
const item = createItem(details, /** @type {TExtraArg} */(arg));
if (item === null) { continue; }
container.appendChild(item);
if (item.nodeType === ELEMENT_NODE) {
Expand Down
54 changes: 31 additions & 23 deletions ext/js/display/display.js
Original file line number Diff line number Diff line change
Expand Up @@ -590,14 +590,14 @@ export class Display extends EventDispatcher {
/** @type {import('display').HistoryState} */
const newState = (
hasState ?
clone(state) :
{
focusEntry: 0,
optionsContext: void 0,
url: window.location.href,
sentence: {text: query, offset: 0},
documentTitle: document.title,
}
clone(state) :
{
focusEntry: 0,
optionsContext: void 0,
url: window.location.href,
sentence: {text: query, offset: 0},
documentTitle: document.title,
}
);
if (!hasState || updateOptionsContext) {
newState.optionsContext = clone(this._optionsContext);
Expand Down Expand Up @@ -733,6 +733,7 @@ export class Display extends EventDispatcher {

/** @type {import('display').DirectApiHandler<'displaySetContent'>} */
_onMessageSetContent({details}) {
performance.mark('invokeDisplaySetContent:end');
this.setContent(details);
}

Expand Down Expand Up @@ -817,10 +818,10 @@ export class Display extends EventDispatcher {
this._queryParserVisibleOverride = (fullVisible === null ? null : (fullVisible !== 'false'));

this._historyHasChanged = true;
performance.mark('display:prepare:end');
performance.measure('display:prepare', 'display:prepare:start', 'display:prepare:end');
performance.mark('display:_onStateChanged:prepare:end');
performance.measure('display:_onStateChanged:prepare', 'display:_onStateChanged:prepare:start', 'display:_onStateChanged:prepare:end');

performance.mark('display:setContent:start');
performance.mark('display:_onStateChanged:setContent:start');
// Set content
switch (type) {
case 'terms':
Expand All @@ -837,13 +838,13 @@ export class Display extends EventDispatcher {
this._clearContent();
break;
}
performance.mark('display:setContent:end');
performance.measure('display:setContent', 'display:setContent:start', 'display:setContent:end');
performance.mark('display:_onStateChanged:setContent:end');
performance.measure('display:_onStateChanged:setContent', 'display:_onStateChanged:setContent:start', 'display:_onStateChanged:setContent:end');
} catch (e) {
this.onError(toError(e));
}
performance.mark('display:onStateChanged:end');
performance.measure('display:onStateChanged', 'display:onStateChanged:start', 'display:onStateChanged:end');
performance.mark('display:_onStateChanged:end');
performance.measure('display:_onStateChanged', 'display:_onStateChanged:start', 'display:_onStateChanged:end');
}

/**
Expand All @@ -856,8 +857,8 @@ export class Display extends EventDispatcher {
eventType === 'click' ||
!(typeof historyState === 'object' && historyState !== null) ||
historyState.cause !== 'queryParser' ?
'new' :
'overwrite'
'new' :
'overwrite'
);
/** @type {import('display').ContentDetails} */
const details = {
Expand Down Expand Up @@ -1363,7 +1364,10 @@ export class Display extends EventDispatcher {

let {dictionaryEntries} = content;
if (!Array.isArray(dictionaryEntries)) {
performance.mark('display:findDictionaryEntries:start');
dictionaryEntries = hasEnabledDictionaries && lookup && query.length > 0 ? await this._findDictionaryEntries(type === 'kanji', query, primaryReading, wildcardsEnabled, optionsContext) : [];
performance.mark('display:findDictionaryEntries:end');
performance.measure('display:findDictionaryEntries', 'display:findDictionaryEntries:start', 'display:findDictionaryEntries:end');
if (this._setContentToken !== token) { return; }
content.dictionaryEntries = dictionaryEntries;
changeHistory = true;
Expand Down Expand Up @@ -1398,7 +1402,11 @@ export class Display extends EventDispatcher {

this._dictionaryEntries = dictionaryEntries;

performance.mark('display:updateNavigationAuto:start');
this._updateNavigationAuto();
performance.mark('display:updateNavigationAuto:end');
performance.measure('display:updateNavigationAuto', 'display:updateNavigationAuto:start', 'display:updateNavigationAuto:end');

this._setNoContentVisible(hasEnabledDictionaries && dictionaryEntries.length === 0 && lookup);
this._setNoDictionariesVisible(!hasEnabledDictionaries);

Expand Down Expand Up @@ -1734,8 +1742,8 @@ export class Display extends EventDispatcher {
_relativeTermView(next) {
return (
next ?
this._history.hasNext() && this._history.forward() :
this._history.hasPrevious() && this._history.back()
this._history.hasNext() && this._history.forward() :
this._history.hasPrevious() && this._history.back()
);
}

Expand Down Expand Up @@ -1820,8 +1828,8 @@ export class Display extends EventDispatcher {
_isQueryParserVisible() {
return (
this._queryParserVisibleOverride !== null ?
this._queryParserVisibleOverride :
this._queryParserVisible
this._queryParserVisibleOverride :
this._queryParserVisible
);
}

Expand Down Expand Up @@ -1859,8 +1867,8 @@ export class Display extends EventDispatcher {
this._childrenSupported &&
(
(isSearchPage) ?
(options.scanning.enableOnSearchPage) :
(this._depth < options.scanning.popupNestingMaxDepth)
(options.scanning.enableOnSearchPage) :
(this._depth < options.scanning.popupNestingMaxDepth)
)
);

Expand Down
Loading
Loading