Merge pull request #12188 from keymanapp/refactor/web/drop-interface-…

…jsproc-dependency

web/README.md

@@ -81,11 +81,11 @@ title: Dependency Graph
 %%{init: {"flowchart": {"htmlLabels": false}} }%%
 graph TD;
     OSK["/web/src/engine/osk"];
-    KP["/web/src/engine/keyboard"];
+    KeyboardSpec["/web/src/engine/keyboard"];
     JSProc["/web/src/engine/js-processor"];
-    OSK-->KP;
+    OSK-->KeyboardSpec;
     WebUtils["@keymanapp/web-utils<br>(/common/web/utils)"];
-    KP---->WebUtils;
+    KeyboardSpec---->WebUtils;
     Wordbreakers["@keymanapp/models-wordbreakers<br>(/common/models/wordbreakers)"];
     Models["@keymanapp/models-templates<br>(/common/models/templates)"];
     Models-->WebUtils;
@@ -107,15 +107,15 @@ graph TD;
     subgraph Headless["`**Headless**
     Fully headless components`"]
         direction LR
-        KP;
-        JSProc-->KP;
+        KeyboardSpec;
+        JSProc-->KeyboardSpec;
         WebUtils;
         PredText;
         Gestures;
     end
 
     subgraph ClassicWeb["`**ClassicWeb**
-    Previously unmodularized components`"]
+    Intermediate-level engine modules`"]
         Device["/web/src/engine/device-detect"];
         Device----->WebUtils;
         Elements["/web/src/engine/element-wrappers"];
@@ -124,12 +124,11 @@ graph TD;
         KeyboardStorage-->Interfaces;
         DomUtils["/web/src/engine/dom-utils"];
         DomUtils-->WebUtils;
-        DomUtils-->KP;
+        DomUtils-->KeyboardSpec;
         OSK-->DomUtils;
         OSK-->Gestures;
         Interfaces["/web/src/engine/interfaces"];
-        Interfaces-->KP;
-        Interfaces-->JSProc;
+        Interfaces-->KeyboardSpec;
         OSK-->Interfaces;
         CommonEngine["/web/src/engine/main"];
         CommonEngine-->Device;

web/build.sh

@@ -143,6 +143,9 @@ coverage_action() {
 
 builder_run_child_actions build:engine/device-detect
 builder_run_child_actions build:engine/dom-utils
+
+builder_run_child_actions build:engine/keyboard
+builder_run_child_actions build:engine/js-processor
 builder_run_child_actions build:engine/element-wrappers
 builder_run_child_actions build:engine/events
 builder_run_child_actions build:engine/interfaces

web/src/engine/interfaces/src/prediction/languageProcessor.interface.ts

@@ -1,7 +1,7 @@
 ///<reference types="@keymanapp/models-types" />
 
 import { EventEmitter } from "eventemitter3";
-import { OutputTarget } from "keyman/engine/js-processor";
+import { OutputTarget } from "keyman/engine/keyboard";
 
 export class ReadySuggestions {
   suggestions: Suggestion[];

web/src/engine/interfaces/src/prediction/predictionContext.ts

@@ -1,6 +1,6 @@
 import { EventEmitter } from "eventemitter3";
 import { type LanguageProcessorSpec , ReadySuggestions, type InvalidateSourceEnum, StateChangeHandler } from './languageProcessor.interface.js';
-import { type KeyboardProcessor, type OutputTarget } from 'keyman/engine/js-processor';
+import { type OutputTarget } from "keyman/engine/keyboard";
 
 interface PredictionContextEventMap {
   update: (suggestions: Suggestion[]) => void;
@@ -32,7 +32,7 @@ export default class PredictionContext extends EventEmitter<PredictionContextEve
   private recentRevert: boolean = false;
 
   private langProcessor: LanguageProcessorSpec;
-  private kbdProcessor: KeyboardProcessor;
+  private getLayerId: () => string;
 
   /**
    * Represents the active context used when requesting and applying predictive-text operations.
@@ -59,27 +59,18 @@ export default class PredictionContext extends EventEmitter<PredictionContextEve
   private readonly suggestionApplier: (suggestion: Suggestion) => Promise<Reversion>;
   private readonly suggestionReverter: (reversion: Reversion) => void;
 
-  /**
-   * Handler for post-processing once a suggestion has been applied: calls
-   * into the active keyboard's `begin postKeystroke` entry point.
-   *
-   * Called after the suggestion is applied but _before_ new predictions are
-   * requested based on the resulting context.
-   */
-  private readonly postApplicationHandler: () => void;
-
-  public constructor(langProcessor: LanguageProcessorSpec, kbdProcessor: KeyboardProcessor) {
+  public constructor(langProcessor: LanguageProcessorSpec, getLayerId: () => string) {
     super();
 
     this.langProcessor = langProcessor;
-    this.kbdProcessor = kbdProcessor;
+    this.getLayerId = getLayerId;
 
     const validSuggestionState: () => boolean = () =>
       this.currentTarget && langProcessor.state == 'configured';
 
     this.suggestionApplier = (suggestion) => {
       if(validSuggestionState()) {
-        return langProcessor.applySuggestion(suggestion, this.currentTarget, () => kbdProcessor.layerId);
+        return langProcessor.applySuggestion(suggestion, this.currentTarget, getLayerId);
       } else {
         return null;
       }
@@ -94,19 +85,6 @@ export default class PredictionContext extends EventEmitter<PredictionContextEve
       }
     }
 
-    // As it's called synchronously via event-callback during `this.suggestionApplier`,
-    // `this.currentTarget` is guaranteed to remain unchanged.
-    this.postApplicationHandler = () => {
-      // Tell the keyboard that the current layer has not changed
-      kbdProcessor.newLayerStore.set('');
-      kbdProcessor.oldLayerStore.set('');
-      // Call the keyboard's entry point.
-      kbdProcessor.processPostKeystroke(kbdProcessor.contextDevice, this.currentTarget)
-        // If we have a RuleBehavior as a result, run it on the target. This should
-        // only change system store and variable store values.
-        ?.finalize(kbdProcessor, this.currentTarget, true);
-    };
-
     this.connect();
   }
 
@@ -116,8 +94,6 @@ export default class PredictionContext extends EventEmitter<PredictionContextEve
     this.langProcessor.addListener('tryaccept', this.doTryAccept);
     this.langProcessor.addListener('tryrevert', this.doTryRevert);
     this.langProcessor.addListener('statechange', this.onModelStateChange);
-
-    this.langProcessor.addListener('suggestionapplied', this.postApplicationHandler);
   }
 
   public disconnect() {
@@ -126,8 +102,6 @@ export default class PredictionContext extends EventEmitter<PredictionContextEve
     this.langProcessor.removeListener('tryaccept', this.doTryAccept);
     this.langProcessor.removeListener('tryrevert', this.doTryRevert);
     this.langProcessor.removeListener('statechange', this.onModelStateChange);
-
-    this.langProcessor.removeListener('suggestionapplied', this.postApplicationHandler);
     this.clearSuggestions();
   }
 
@@ -376,7 +350,7 @@ export default class PredictionContext extends EventEmitter<PredictionContextEve
     if(target) {
       // Note:  should be triggered after the corresponding new-context event rule has been processed,
       // as that may affect the value of layerId here.
-      return this.langProcessor.invalidateContext(target, this.kbdProcessor.layerId);
+      return this.langProcessor.invalidateContext(target, this.getLayerId());
     } else {
       return Promise.resolve([]);
     }

web/src/engine/interfaces/tsconfig.json

@@ -13,6 +13,5 @@
 
   "references": [
     { "path": "../keyboard" },
-    { "path": "../js-processor" }
   ]
 }

web/src/engine/js-processor/src/outputTarget.ts

@@ -3,6 +3,7 @@
 import { extendString } from "@keymanapp/web-utils";
 import { findCommonSubstringEndIndex } from "./stringDivergence.js";
 import { Mock } from "./mock.js";
+import { OutputTarget as OutputTargetInterface } from 'keyman/engine/keyboard';
 
 extendString();
 
@@ -66,7 +67,7 @@ export class Transcription {
 
 export type Alternate = ProbabilityMass<Transform>;
 
-export default abstract class OutputTarget {
+export default abstract class OutputTarget implements OutputTargetInterface {
   private _dks: DeadkeyTracker;
 
   constructor() {

web/src/engine/keyboard/src/index.ts

@@ -31,6 +31,7 @@ export * from "./defaultRules.js";
 export { default as KeyEvent } from "./keyEvent.js";
 export * from "./keyEvent.js";
 export { default as KeyMapping } from "./keyMapping.js";
+export { OutputTarget } from "./outputTarget.interface.js";
 
 export * from "@keymanapp/web-utils";
 

web/src/engine/main/src/headless/languageProcessor.ts

@@ -336,6 +336,7 @@ export class LanguageProcessor extends EventEmitter<LanguageProcessorEventMap> {
 
   public shutdown() {
     this.lmEngine.shutdown();
+    this.removeAllListeners();
   }
 
   public get isActive(): boolean {

web/src/engine/main/src/keymanEngine.ts

@@ -1,5 +1,5 @@
 import { type KeyEvent, type Keyboard, KeyboardKeymanGlobal } from "keyman/engine/keyboard";
-import { ProcessorInitOptions, RuleBehavior } from 'keyman/engine/js-processor';
+import { OutputTarget, ProcessorInitOptions, RuleBehavior } from 'keyman/engine/js-processor';
 import { DOMKeyboardLoader as KeyboardLoader } from "keyman/engine/keyboard/dom-keyboard-loader";
 import { InputProcessor } from './headless/inputProcessor.js';
 import { OSKView } from "keyman/engine/osk";
@@ -241,6 +241,8 @@ export default class KeymanEngine<
     this.modelCache = new ModelCache();
     const kbdCache = this.keyboardRequisitioner.cache;
 
+    const keyboardProcessor = this.core.keyboardProcessor;
+    const predictionContext = new PredictionContext(this.core.languageProcessor, () => keyboardProcessor.layerId);
     this.contextManager.configure({
       resetContext: (target) => {
         // Could reset the target's deadkeys here, but it's really more of a 'core' task.
@@ -253,10 +255,27 @@ export default class KeymanEngine<
           this.core.resetContext(target);
         }
       },
-      predictionContext: new PredictionContext(this.core.languageProcessor, this.core.keyboardProcessor),
+      predictionContext: predictionContext,
       keyboardCache: this.keyboardRequisitioner.cache
     });
 
+    /*
+     * Handler for post-processing once a suggestion has been applied.
+     *
+     * This is called after the suggestion is applied but _before_ new
+     * predictions are requested based on the resulting context.
+     */
+    this.core.languageProcessor.on('suggestionapplied', () => {
+      // Tell the keyboard that the current layer has not changed
+      keyboardProcessor.newLayerStore.set('');
+      keyboardProcessor.oldLayerStore.set('');
+      // Call the keyboard's entry point.
+      keyboardProcessor.processPostKeystroke(keyboardProcessor.contextDevice, predictionContext.currentTarget as OutputTarget)
+        // If we have a RuleBehavior as a result, run it on the target. This should
+        // only change system store and variable store values.
+        ?.finalize(keyboardProcessor, predictionContext.currentTarget as OutputTarget, true);
+    });
+
     // #region Event handler wiring
     this.config.on('spacebartext', () => {
       // On change of spacebar-text mode, we currently need a layout refresh to update the

web/src/test/auto/headless/engine/interfaces/prediction/predictionContext.spec.js

@@ -5,7 +5,7 @@ import { LanguageProcessor, TranscriptionCache } from 'keyman/engine/main';
 import { PredictionContext } from 'keyman/engine/interfaces';
 import { Worker as LMWorker } from "@keymanapp/lexical-model-layer/node";
 import { DeviceSpec } from 'keyman/engine/keyboard';
-import { KeyboardProcessor, Mock } from 'keyman/engine/js-processor';
+import { Mock } from 'keyman/engine/js-processor';
 
 function compileDummyModel(suggestionSets) {
   return `
@@ -15,14 +15,6 @@ LMLayerWorker.loadModel(new models.DummyModel({
 `;
 }
 
-// Common spec used for each test's setup.  It's actually irrelevant for the tests,
-// but KeyboardProcessor needs an instance.
-const deviceSpec = new DeviceSpec(
-  DeviceSpec.Browser.Chrome,
-  DeviceSpec.FormFactor.Desktop,
-  DeviceSpec.OperatingSystem.Windows
-);
-
 const appleDummySuggestionSets = [[
   // Set 1:
   {
@@ -67,6 +59,10 @@ const appleDummyModel = {
   code: compileDummyModel(appleDummySuggestionSets)
 };
 
+function dummiedGetLayer() {
+  return 'default';
+}
+
 describe("PredictionContext", () => {
   let worker;
 
@@ -82,8 +78,7 @@ describe("PredictionContext", () => {
     const langProcessor = new LanguageProcessor(worker, new TranscriptionCache());
     await langProcessor.loadModel(appleDummyModel);  // await:  must fully 'configure', load script into worker.
 
-    const kbdProcessor = new KeyboardProcessor(deviceSpec);
-    const predictiveContext = new PredictionContext(langProcessor, kbdProcessor);
+    const predictiveContext = new PredictionContext(langProcessor, dummiedGetLayer);
 
     let updateFake = sinon.fake();
     predictiveContext.on('update', updateFake);
@@ -108,7 +103,7 @@ describe("PredictionContext", () => {
 
     mock.insertTextBeforeCaret('e'); // appl| + e = apple
     let transcription = mock.buildTranscriptionFrom(initialMock, null, true);
-    await langProcessor.predict(transcription, kbdProcessor.layerId);
+    await langProcessor.predict(transcription, dummiedGetLayer());
 
     // First predict call results:  our second set of dummy suggestions, the first of which includes
     // a 'keep' of the original text.
@@ -123,8 +118,7 @@ describe("PredictionContext", () => {
     const langProcessor = new LanguageProcessor(worker, new TranscriptionCache());
     await langProcessor.loadModel(appleDummyModel);  // await:  must fully 'configure', load script into worker.
 
-    const kbdProcessor = new KeyboardProcessor(deviceSpec);
-    const predictiveContext = new PredictionContext(langProcessor, kbdProcessor);
+    const predictiveContext = new PredictionContext(langProcessor, dummiedGetLayer);
 
     let updateFake = sinon.fake();
     predictiveContext.on('update', updateFake);
@@ -151,13 +145,13 @@ describe("PredictionContext", () => {
 
     // Mocking:  corresponds to the second set of mocked predictions - round 2 of
     // 'apple', 'apply', 'apples'.
-    const skippedPromise = langProcessor.predict(baseTranscription, kbdProcessor.layerId);
+    const skippedPromise = langProcessor.predict(baseTranscription, dummiedGetLayer());
 
     mock.insertTextBeforeCaret('e'); // appl| + e = apple
     const finalTranscription = mock.buildTranscriptionFrom(initialMock, null, true);
 
     // Mocking:  corresponds to the third set of mocked predictions - 'applied'.
-    const expectedPromise = langProcessor.predict(finalTranscription, kbdProcessor.layerId);
+    const expectedPromise = langProcessor.predict(finalTranscription, dummiedGetLayer());
 
     await Promise.all([skippedPromise, expectedPromise]);
     const expected = await expectedPromise;
@@ -182,8 +176,7 @@ describe("PredictionContext", () => {
     const langProcessor = new LanguageProcessor(worker, new TranscriptionCache());
     await langProcessor.loadModel(appleDummyModel);  // await:  must fully 'configure', load script into worker.
 
-    const kbdProcessor = new KeyboardProcessor(deviceSpec);
-    const predictiveContext = new PredictionContext(langProcessor, kbdProcessor);
+    const predictiveContext = new PredictionContext(langProcessor, dummiedGetLayer);
 
     let mock = new Mock("appl", 4); // "appl|", with '|' as the caret position.
     const initialSuggestions = await predictiveContext.setCurrentTarget(mock);
@@ -208,8 +201,7 @@ describe("PredictionContext", () => {
     const langProcessor = new LanguageProcessor(worker, new TranscriptionCache());
     await langProcessor.loadModel(appleDummyModel);  // await:  must fully 'configure', load script into worker.
 
-    const kbdProcessor = new KeyboardProcessor(deviceSpec);
-    const predictiveContext = new PredictionContext(langProcessor, kbdProcessor);
+    const predictiveContext = new PredictionContext(langProcessor, dummiedGetLayer);
 
     let textState = new Mock("appl", 4); // "appl|", with '|' as the caret position.
 
@@ -223,7 +215,7 @@ describe("PredictionContext", () => {
     let previousTextState = Mock.from(textState);
     textState.insertTextBeforeCaret('e'); // appl| + e = apple
     let transcription = textState.buildTranscriptionFrom(previousTextState, null, true);
-    await langProcessor.predict(transcription, kbdProcessor.layerId);
+    await langProcessor.predict(transcription, dummiedGetLayer());
 
     // Verify setup.
     assert.equal(updateFake.callCount, 1);
@@ -276,8 +268,7 @@ describe("PredictionContext", () => {
     const langProcessor = new LanguageProcessor(worker, new TranscriptionCache());
     await langProcessor.loadModel(appleDummyModel);  // await:  must fully 'configure', load script into worker.
 
-    const kbdProcessor = new KeyboardProcessor(deviceSpec);
-    const predictiveContext = new PredictionContext(langProcessor, kbdProcessor);
+    const predictiveContext = new PredictionContext(langProcessor, dummiedGetLayer);
 
     let textState = new Mock("appl", 4); // "appl|", with '|' as the caret position.
 
@@ -292,7 +283,7 @@ describe("PredictionContext", () => {
 
     let suggestionCaptureFake = sinon.fake();
     predictiveContext.once('update', suggestionCaptureFake);
-    await langProcessor.predict(transcription, kbdProcessor.layerId);
+    await langProcessor.predict(transcription, dummiedGetLayer());
 
     // We need to capture the suggestion we wish to apply.  We could hardcode a forced
     // value, but that might become brittle in the long-term.