From baf2150542cfc03fc03a0b783fdce49b139eacab Mon Sep 17 00:00:00 2001 From: Joshua Horton Date: Thu, 20 Feb 2025 12:06:28 +0700 Subject: [PATCH 1/4] fix(web): properly support workerless use of Web engine Fixes: #13262 Fixes: KEYMANWEB-COM-1VH --- web/src/app/browser/src/debug-main.ts | 2 +- web/src/app/browser/src/keymanEngine.ts | 5 +- web/src/app/browser/src/release-main.ts | 2 +- web/src/app/webview/src/debug-main.ts | 2 +- web/src/app/webview/src/keymanEngine.ts | 5 +- web/src/app/webview/src/release-main.ts | 2 +- .../main/src/headless/inputProcessor.ts | 5 +- .../main/src/headless/languageProcessor.ts | 58 ++++++++++++++----- web/src/engine/main/src/keymanEngine.ts | 5 +- .../predictive-text/worker-main/src/index.ts | 6 +- .../worker-main/src/node/index.ts | 6 +- .../worker-main/src/web/index.ts | 6 +- 12 files changed, 75 insertions(+), 29 deletions(-) diff --git a/web/src/app/browser/src/debug-main.ts b/web/src/app/browser/src/debug-main.ts index 2613cf3a11..4a2096d3d3 100644 --- a/web/src/app/browser/src/debug-main.ts +++ b/web/src/app/browser/src/debug-main.ts @@ -13,4 +13,4 @@ import { SourcemappedWorker } from '@keymanapp/lexical-model-layer/web' var sPath = ss.substr(0,ss.lastIndexOf('/')+1); // @ts-ignore -window['keyman'] = new KeymanEngine(SourcemappedWorker.constructInstance(), sPath); \ No newline at end of file +window['keyman'] = new KeymanEngine(SourcemappedWorker, sPath); \ No newline at end of file diff --git a/web/src/app/browser/src/keymanEngine.ts b/web/src/app/browser/src/keymanEngine.ts index 3c30740e81..454a2b6620 100644 --- a/web/src/app/browser/src/keymanEngine.ts +++ b/web/src/app/browser/src/keymanEngine.ts @@ -27,6 +27,7 @@ import { UIModule } from './uiModuleInterface.js'; import { HotkeyManager } from './hotkeyManager.js'; import { BeepHandler } from './beepHandler.js'; import KeyboardInterface from './keyboardInterface.js'; +import { WorkerFactory } from '@keymanapp/lexical-model-layer/web'; export default class KeymanEngine extends KeymanEngineBase { touchLanguageMenu?: LanguageMenu; @@ -55,10 +56,10 @@ export default class KeymanEngine extends KeymanEngineBase this.legacyAPIEvents), (engine) => { + super(workerFactory, config, new ContextManager(config, () => this.legacyAPIEvents), (engine) => { return { // The `engine` parameter cannot be supplied with the constructing instance before calling // `super`, hence the 'fun' rigging to supply it _from_ `super` via this closure. diff --git a/web/src/app/browser/src/release-main.ts b/web/src/app/browser/src/release-main.ts index 62fefb5ad2..01b318c714 100644 --- a/web/src/app/browser/src/release-main.ts +++ b/web/src/app/browser/src/release-main.ts @@ -13,4 +13,4 @@ import { Worker } from '@keymanapp/lexical-model-layer/web' var sPath = ss.substr(0,ss.lastIndexOf('/')+1); // @ts-ignore -window['keyman'] = new KeymanEngine(Worker.constructInstance(), sPath); \ No newline at end of file +window['keyman'] = new KeymanEngine(Worker, sPath); \ No newline at end of file diff --git a/web/src/app/webview/src/debug-main.ts b/web/src/app/webview/src/debug-main.ts index d903a9d536..3e7c7270ec 100644 --- a/web/src/app/webview/src/debug-main.ts +++ b/web/src/app/webview/src/debug-main.ts @@ -13,4 +13,4 @@ const ss = scripts[scripts.length-1].src; const sPath = ss.substring(0,ss.lastIndexOf('/')+1); // @ts-ignore -window['keyman'] = new KeymanEngine(SourcemappedWorker.constructInstance(), sPath); \ No newline at end of file +window['keyman'] = new KeymanEngine(SourcemappedWorker, sPath); \ No newline at end of file diff --git a/web/src/app/webview/src/keymanEngine.ts b/web/src/app/webview/src/keymanEngine.ts index d390a6ce1b..1d7088f7ed 100644 --- a/web/src/app/webview/src/keymanEngine.ts +++ b/web/src/app/webview/src/keymanEngine.ts @@ -9,12 +9,13 @@ import { WebviewConfiguration, WebviewInitOptionDefaults, WebviewInitOptionSpec import ContextManager, { ContextHost } from './contextManager.js'; import PassthroughKeyboard from './passthroughKeyboard.js'; import { buildEmbeddedGestureConfig, setupEmbeddedListeners } from './oskConfiguration.js'; +import { WorkerFactory } from '@keymanapp/lexical-model-layer'; export default class KeymanEngine extends KeymanEngineBase { // Ideally, we would be able to auto-detect `sourceUri`: https://stackoverflow.com/a/60244278. // But it's too new of a feature to utilize... and also expects to be in a module, when this may // be compiled down to an IIFE. - constructor(worker: Worker, sourceUri: string) { + constructor(workerFactory: WorkerFactory, sourceUri: string) { const config = new WebviewConfiguration(sourceUri); // currently set to perform device auto-detect. config.onRuleFinalization = (ruleBehavior: RuleBehavior) => { @@ -30,7 +31,7 @@ export default class KeymanEngine extends KeymanEngineBase { + super(workerFactory, config, new ContextManager(config), (engine) => { return { // The `engine` parameter cannot be supplied with the constructing instance before calling // `super`, hence the 'fun' rigging to supply it _from_ `super` via this closure. diff --git a/web/src/app/webview/src/release-main.ts b/web/src/app/webview/src/release-main.ts index e594e0b70f..fd8bcd8dce 100644 --- a/web/src/app/webview/src/release-main.ts +++ b/web/src/app/webview/src/release-main.ts @@ -13,4 +13,4 @@ const ss = scripts[scripts.length-1].src; const sPath = ss.substring(0,ss.lastIndexOf('/')+1); // @ts-ignore -window['keyman'] = new KeymanEngine(Worker.constructInstance(), sPath); \ No newline at end of file +window['keyman'] = new KeymanEngine(Worker, sPath); \ No newline at end of file diff --git a/web/src/engine/main/src/headless/inputProcessor.ts b/web/src/engine/main/src/headless/inputProcessor.ts index c13a0bf00a..2a21ec2b92 100644 --- a/web/src/engine/main/src/headless/inputProcessor.ts +++ b/web/src/engine/main/src/headless/inputProcessor.ts @@ -20,6 +20,7 @@ import { import { TranscriptionCache } from "./transcriptionCache.js"; import { LexicalModelTypes } from '@keymanapp/common-types'; +import { WorkerFactory } from "@keymanapp/lexical-model-layer"; export class InputProcessor { public static readonly DEFAULT_OPTIONS: ProcessorInitOptions = { @@ -37,7 +38,7 @@ export class InputProcessor { private readonly contextCache = new TranscriptionCache(); - constructor(device: DeviceSpec, predictiveTextWorker: Worker, options?: ProcessorInitOptions) { + constructor(device: DeviceSpec, predictiveWorkerFactory: WorkerFactory, options?: ProcessorInitOptions) { if(!device) { throw new Error('device must be defined'); } @@ -48,7 +49,7 @@ export class InputProcessor { this.contextDevice = device; this.kbdProcessor = new KeyboardProcessor(device, options); - this.lngProcessor = new LanguageProcessor(predictiveTextWorker, this.contextCache); + this.lngProcessor = new LanguageProcessor(predictiveWorkerFactory, this.contextCache); } public get languageProcessor(): LanguageProcessor { diff --git a/web/src/engine/main/src/headless/languageProcessor.ts b/web/src/engine/main/src/headless/languageProcessor.ts index 6783e344c9..f23bccd527 100644 --- a/web/src/engine/main/src/headless/languageProcessor.ts +++ b/web/src/engine/main/src/headless/languageProcessor.ts @@ -1,5 +1,5 @@ import { EventEmitter } from "eventemitter3"; -import { LMLayer } from "@keymanapp/lexical-model-layer/web"; +import { LMLayer, WorkerFactory } from "@keymanapp/lexical-model-layer/web"; import { OutputTarget, Transcription, Mock } from "keyman/engine/js-processor"; import { LanguageProcessorEventMap, ModelSpec, StateChangeEnum, ReadySuggestions } from 'keyman/engine/interfaces'; import ContextWindow from "./contextWindow.js"; @@ -25,7 +25,7 @@ export class LanguageProcessor extends EventEmitter { private _state: StateChangeEnum = 'inactive'; - public constructor(predictiveTextWorker: Worker, transcriptionCache: TranscriptionCache, supportsRightDeletions: boolean = false) { + public constructor(predictiveWorkerFactory: WorkerFactory, transcriptionCache: TranscriptionCache, supportsRightDeletions: boolean = false) { super(); this.recentTranscriptions = transcriptionCache; @@ -38,11 +38,15 @@ export class LanguageProcessor extends EventEmitter { maxRightContextCodePoints: supportsRightDeletions ? 0 : 64 } - if(!predictiveTextWorker) { + if(!predictiveWorkerFactory) { return; } - this.lmEngine = new LMLayer(capabilities, predictiveTextWorker); + try { + this.lmEngine = new LMLayer(capabilities, predictiveWorkerFactory?.constructInstance()); + } catch { + // We can condition on `lmEngine` being null/undefined. + } } public get activeModel(): ModelSpec { @@ -58,6 +62,10 @@ export class LanguageProcessor extends EventEmitter { } public unloadModel() { + if(!this.isActive) { + return; + } + this.lmEngine.unloadModel(); delete this.currentModel; delete this.configuration; @@ -71,6 +79,10 @@ export class LanguageProcessor extends EventEmitter { throw new Error("Null reference not allowed."); } + if(!this.canEnable) { + return Promise.resolve(); + } + let specType: 'file'|'raw' = model.path ? 'file' : 'raw'; let source = specType == 'file' ? model.path : model.code; @@ -108,9 +120,6 @@ export class LanguageProcessor extends EventEmitter { } public invalidateContext(outputTarget: OutputTarget, layerId: string): Promise { - // Signal to any predictive text UI that the context has changed, invalidating recent predictions. - this.emit('invalidatesuggestions', 'context'); - // If there's no active model, there can be no predictions. // We'll also be missing important data needed to even properly REQUEST the predictions. if(!this.currentModel || !this.configuration) { @@ -121,7 +130,12 @@ export class LanguageProcessor extends EventEmitter { // invalidateContext otherwise bypasses .predict()'s check against this. if(!this.isActive) { return Promise.resolve([]); - } else if(outputTarget) { + } + + // Signal to any predictive text UI that the context has changed, invalidating recent predictions. + this.emit('invalidatesuggestions', 'context'); + + if(outputTarget) { let transcription = outputTarget.buildTranscriptionFrom(outputTarget, null, false); return this.predict_internal(transcription, true, layerId); } else { @@ -173,6 +187,10 @@ export class LanguageProcessor extends EventEmitter { throw "Accepting suggestions requires a destination OutputTarget instance." } + if(!this.isActive) { + return null; + } + if(!this.isConfigured) { // If we're in this state, the suggestion is now outdated; the user must have swapped keyboard and model. console.warn("Could not apply suggestion; the corresponding model has been unloaded"); @@ -244,6 +262,10 @@ export class LanguageProcessor extends EventEmitter { throw "Accepting suggestions requires a destination OutputTarget instance." } + if(!this.isActive) { + return null; + } + // Find the state of the context at the time the suggestion was generated. // This may refer to the context before an input keystroke or before application // of a predictive suggestion. @@ -278,7 +300,7 @@ export class LanguageProcessor extends EventEmitter { } public predictFromTarget(outputTarget: OutputTarget, layerId: string): Promise { - if(!outputTarget) { + if(!this.isActive || !outputTarget) { return null; } @@ -292,7 +314,7 @@ export class LanguageProcessor extends EventEmitter { * @param transcription The triggering transcription (if it exists) */ private predict_internal(transcription: Transcription, resetContext: boolean, layerId: string): Promise { - if(!transcription) { + if(!this.isActive || !transcription) { return null; } @@ -341,19 +363,19 @@ export class LanguageProcessor extends EventEmitter { } public shutdown() { - this.lmEngine.shutdown(); + this.lmEngine?.shutdown(); this.removeAllListeners(); } public get isActive(): boolean { - if(!this.canEnable()) { + if(!this.canEnable) { this._mayPredict = false; return false; } return (this.activeModel || false) && this._mayPredict; } - canEnable(): boolean { + public get canEnable(): boolean { // Is not initialized if there is no worker. return !!this.lmEngine; } @@ -363,7 +385,7 @@ export class LanguageProcessor extends EventEmitter { } public set mayPredict(flag: boolean) { - if(!this.canEnable()) { + if(!this.canEnable) { return; } @@ -416,6 +438,10 @@ export class LanguageProcessor extends EventEmitter { } public tryAcceptSuggestion(source: string): boolean { + if(!this.isActive) { + return false; + } + // The object below is to facilitate a pass-by-reference on the boolean flag, // allowing the event's handler to signal if whitespace has been added via // auto-applied suggestion that should be blocked on the next keystroke. @@ -426,6 +452,10 @@ export class LanguageProcessor extends EventEmitter { } public tryRevertSuggestion(): boolean { + if(!this.isActive) { + return false; + } + // If and when we do auto-revert, the suggestion is to pass this object to the event and // denote any mutations to the contained value. //let returnObj = {shouldSwallow: false}; diff --git a/web/src/engine/main/src/keymanEngine.ts b/web/src/engine/main/src/keymanEngine.ts index beced96d18..8ef53a9008 100644 --- a/web/src/engine/main/src/keymanEngine.ts +++ b/web/src/engine/main/src/keymanEngine.ts @@ -1,6 +1,7 @@ import { type KeyEvent, type Keyboard, KeyboardKeymanGlobal } from "keyman/engine/keyboard"; import { OutputTarget, ProcessorInitOptions, RuleBehavior } from 'keyman/engine/js-processor'; import { DOMKeyboardLoader as KeyboardLoader } from "keyman/engine/keyboard/dom-keyboard-loader"; +import { WorkerFactory } from "@keymanapp/lexical-model-layer/web" import { InputProcessor } from './headless/inputProcessor.js'; import { OSKView } from "keyman/engine/osk"; import { KeyboardRequisitioner, ModelCache, toUnprefixedKeyboardId as unprefixed } from "keyman/engine/keyboard-storage"; @@ -112,7 +113,7 @@ export default class KeymanEngine< * the superclass constructor. */ constructor( - worker: Worker, + workerFactory: WorkerFactory, config: Configuration, contextManager: ContextManager, processorConfigInitializer: (engine: KeymanEngine) => ProcessorConfiguration @@ -123,7 +124,7 @@ export default class KeymanEngine< const processorConfiguration = processorConfigInitializer(this); processorConfiguration.baseLayout = determineBaseLayout(); this.interface = processorConfiguration.keyboardInterface as KeyboardInterface; - this.core = new InputProcessor(config.hostDevice, worker, processorConfiguration); + this.core = new InputProcessor(config.hostDevice, workerFactory, processorConfiguration); this.core.languageProcessor.on('statechange', (state) => { // The banner controller cannot directly trigger a layout-refresh at this time, diff --git a/web/src/engine/predictive-text/worker-main/src/index.ts b/web/src/engine/predictive-text/worker-main/src/index.ts index e087e6957d..98a9dd9101 100644 --- a/web/src/engine/predictive-text/worker-main/src/index.ts +++ b/web/src/engine/predictive-text/worker-main/src/index.ts @@ -1,3 +1,7 @@ export { default as LMLayer } from './lmlayer.js'; export { default as NodeWorker } from './node/worker.js'; -export { default as WebWorker } from './web/worker.js'; \ No newline at end of file +export { default as WebWorker } from './web/worker.js'; + +export interface WorkerFactory { + constructInstance(): Worker +} \ No newline at end of file diff --git a/web/src/engine/predictive-text/worker-main/src/node/index.ts b/web/src/engine/predictive-text/worker-main/src/node/index.ts index 1a70a48bf3..3393c22b81 100644 --- a/web/src/engine/predictive-text/worker-main/src/node/index.ts +++ b/web/src/engine/predictive-text/worker-main/src/node/index.ts @@ -1,3 +1,7 @@ export { default as LMLayer } from '../lmlayer.js'; export { default as Worker } from './worker.js'; -export { default as SourcemappedWorker } from './sourcemappedWorker.js'; \ No newline at end of file +export { default as SourcemappedWorker } from './sourcemappedWorker.js'; + +export interface WorkerFactory { + constructInstance(): Worker + } \ No newline at end of file diff --git a/web/src/engine/predictive-text/worker-main/src/web/index.ts b/web/src/engine/predictive-text/worker-main/src/web/index.ts index 1a70a48bf3..f133f6f2f8 100644 --- a/web/src/engine/predictive-text/worker-main/src/web/index.ts +++ b/web/src/engine/predictive-text/worker-main/src/web/index.ts @@ -1,3 +1,7 @@ export { default as LMLayer } from '../lmlayer.js'; export { default as Worker } from './worker.js'; -export { default as SourcemappedWorker } from './sourcemappedWorker.js'; \ No newline at end of file +export { default as SourcemappedWorker } from './sourcemappedWorker.js'; + +export interface WorkerFactory { + constructInstance(): Worker +} \ No newline at end of file From a5c987116bbba4392009cc51e40a9fe25a5823b6 Mon Sep 17 00:00:00 2001 From: Joshua Horton Date: Thu, 20 Feb 2025 12:11:41 +0700 Subject: [PATCH 2/4] fix(web): patches up affected unit test --- .../interfaces/prediction/predictionContext.tests.js | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/web/src/test/auto/headless/engine/interfaces/prediction/predictionContext.tests.js b/web/src/test/auto/headless/engine/interfaces/prediction/predictionContext.tests.js index 9aa9cd8015..6ec7a0bbd9 100644 --- a/web/src/test/auto/headless/engine/interfaces/prediction/predictionContext.tests.js +++ b/web/src/test/auto/headless/engine/interfaces/prediction/predictionContext.tests.js @@ -64,18 +64,17 @@ function dummiedGetLayer() { } describe("PredictionContext", () => { - let worker; + let langProcessor; beforeEach(function() { - worker = LMWorker.constructInstance(); + langProcessor = new LanguageProcessor(LMWorker, new TranscriptionCache()); }); afterEach(function() { - worker.terminate(); + langProcessor.shutdown(); }); it('receives predictions as they are generated', async function () { - const langProcessor = new LanguageProcessor(worker, new TranscriptionCache()); await langProcessor.loadModel(appleDummyModel); // await: must fully 'configure', load script into worker. const predictiveContext = new PredictionContext(langProcessor, dummiedGetLayer); @@ -115,7 +114,6 @@ describe("PredictionContext", () => { }); it('ignores outdated predictions', async function () { - const langProcessor = new LanguageProcessor(worker, new TranscriptionCache()); await langProcessor.loadModel(appleDummyModel); // await: must fully 'configure', load script into worker. const predictiveContext = new PredictionContext(langProcessor, dummiedGetLayer); @@ -179,7 +177,6 @@ describe("PredictionContext", () => { }); it('sendUpdateState retrieves the most recent suggestion set', async function() { - const langProcessor = new LanguageProcessor(worker, new TranscriptionCache()); await langProcessor.loadModel(appleDummyModel); // await: must fully 'configure', load script into worker. const predictiveContext = new PredictionContext(langProcessor, dummiedGetLayer); @@ -204,7 +201,6 @@ describe("PredictionContext", () => { }); it('suggestion application logic & triggered effects', async function () { - const langProcessor = new LanguageProcessor(worker, new TranscriptionCache()); await langProcessor.loadModel(appleDummyModel); // await: must fully 'configure', load script into worker. const predictiveContext = new PredictionContext(langProcessor, dummiedGetLayer); @@ -271,7 +267,6 @@ describe("PredictionContext", () => { }); it('reversion application logic & triggered effects', async function () { - const langProcessor = new LanguageProcessor(worker, new TranscriptionCache()); await langProcessor.loadModel(appleDummyModel); // await: must fully 'configure', load script into worker. const predictiveContext = new PredictionContext(langProcessor, dummiedGetLayer); From accdb3752e9e618bfbae552f5315f62354eb24d6 Mon Sep 17 00:00:00 2001 From: Joshua Horton Date: Mon, 24 Feb 2025 09:15:50 +0700 Subject: [PATCH 3/4] change(web): apply focused try-catch suggestion from review Co-authored-by: Marc Durdin --- web/src/engine/main/src/headless/languageProcessor.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/web/src/engine/main/src/headless/languageProcessor.ts b/web/src/engine/main/src/headless/languageProcessor.ts index f23bccd527..86e492b726 100644 --- a/web/src/engine/main/src/headless/languageProcessor.ts +++ b/web/src/engine/main/src/headless/languageProcessor.ts @@ -42,10 +42,16 @@ export class LanguageProcessor extends EventEmitter { return; } + let instance: ; try { - this.lmEngine = new LMLayer(capabilities, predictiveWorkerFactory?.constructInstance()); - } catch { + instance = predictiveWorkerFactory?.constructInstance(); + } catch(e) { // We can condition on `lmEngine` being null/undefined. + console.warn('Web workers are not available: ' + (e ?? '').toString()); + instance = null; + } + if(instance) { + this.lmEngine = new LMLayer(capabilities, instance); } } From 72a9e8b8fecc34566461765d5b23fedc4aa8d4fb Mon Sep 17 00:00:00 2001 From: Joshua Horton Date: Mon, 24 Feb 2025 13:06:17 +0700 Subject: [PATCH 4/4] fix(web): fix up applied suggestion - properly "fill in" Worker type --- web/src/engine/main/src/headless/languageProcessor.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/web/src/engine/main/src/headless/languageProcessor.ts b/web/src/engine/main/src/headless/languageProcessor.ts index 86e492b726..b4b1f74797 100644 --- a/web/src/engine/main/src/headless/languageProcessor.ts +++ b/web/src/engine/main/src/headless/languageProcessor.ts @@ -42,16 +42,16 @@ export class LanguageProcessor extends EventEmitter { return; } - let instance: ; + let workerInstance: Worker; try { - instance = predictiveWorkerFactory?.constructInstance(); + workerInstance = predictiveWorkerFactory?.constructInstance(); } catch(e) { // We can condition on `lmEngine` being null/undefined. console.warn('Web workers are not available: ' + (e ?? '').toString()); - instance = null; + workerInstance = null; } - if(instance) { - this.lmEngine = new LMLayer(capabilities, instance); + if(workerInstance) { + this.lmEngine = new LMLayer(capabilities, workerInstance); } }