refactor(web): refactor `ProcessorInitOptions` 📏 🎼 by ermshiperete · Pull Request #15465 · keymanapp/keyman

ermshiperete · 2026-01-16T17:16:19Z

This addresses a web-core TODO comment.

This change makes the options parameter in the JSKeyboardProcessor constructor no longer optional. Also baseLayout and defaultOutputRules are now required in ProcessorInitOptions. For testing purposes we export DEFAULT_OPTIONS in a testing endpoint.

The options parameter in the InputProcessor constructor is no longer optional. Also removes the DEFAULT_OPTIONS from
InputProcessor.

This change also moves the ProcessorInitOptions interface to a separate file and makes keyboardInterface a required field.

An alternative approach would have been to get rid of ProcessorInitOptions and instead set the JSKeyboardProcessor fields after creating the instance. Doing it the implemented way seemed cleaner.

Test-bot: skip

… c'tor This makes the `options` parameter in the `JSKeyboardProcessor` constructor no longer optional. Also `baseLayout` and `defaultOutputRules` are now required in `ProcessorInitOptions`. For testing purposes we export `DEFAULT_OPTIONS` in a testing endpoint. Test-bot: skip

This makes the `options` parameter in the `InputProcessor` constructor no longer optional. Also removes the `DEFAULT_OPTIONS` from `InputProcessor`. Test-bot: skip

This change moves the `ProcessorInitOptions` interface to a separate file and makes `keyboardInterface` a required field. Test-bot: skip

keymanapp-test-bot · 2026-01-16T17:16:24Z

User Test Results

Test specification and instructions

User tests are not required

Test Artifacts

Android
- Keyman for Android apk - build : all tests passed (no artifacts on BuildLevel "build")
- FirstVoices Keyboards for Android apk - build : all tests passed (no artifacts on BuildLevel "build")
- FirstVoices Keyboards for Android apk (old PRs) - build : all tests passed (no artifacts on BuildLevel "build")
- KeyboardHarness apk - build : all tests passed (no artifacts on BuildLevel "build")
- Keyman for Android apk (old PRs) - build : all tests passed (no artifacts on BuildLevel "build")
- KMSample1 apk - build : all tests passed (no artifacts on BuildLevel "build")
- KMSample2 apk - build : all tests passed (no artifacts on BuildLevel "build")
Developer
- Keyman Developer - build : all tests passed (no artifacts on BuildLevel "build")
- Compiler Regression Tests - build : all tests passed (no artifacts on BuildLevel "build")
- Keyman Developer (old PRs) - build : all tests passed (no artifacts on BuildLevel "build")
- kmcomp.zip - build : all tests passed (no artifacts on BuildLevel "build")
- kmcomp.zip (old PRs) - build : all tests passed (no artifacts on BuildLevel "build")
iOS
- Keyman for iOS (simulator image) - build : all tests passed (no artifacts on BuildLevel "build")
- FirstVoices Keyboards for iOS (simulator image) - build : all tests passed (no artifacts on BuildLevel "build")
- FirstVoices Keyboards for iOS (simulator image) (old PRs) - build : all tests passed (no artifacts on BuildLevel "build")
- Keyman for iOS (simulator image) (old PRs) - build : all tests passed (no artifacts on BuildLevel "build")
Keyboards
- Test Keyboards - build : all tests passed (no artifacts on BuildLevel "build")
Web
- KeymanWeb Test Home - build : all tests passed (no artifacts on BuildLevel "build")

mcdurdin

You've refactored the redundant DEFAULT_OPTIONS this far, I think we can take it one step further into a .tests.ts file so it doesn't get constructed at all except for unit tests.

mcdurdin · 2026-01-19T19:09:58Z

        // 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.


I really don't understand this comment!

mcdurdin · 2026-01-19T19:11:46Z

+  DEFAULT_OPTIONS: {
+    baseLayout: 'us',
+    keyboardInterface:  new JSKeyboardInterface(globalObject(), MinimalKeymanGlobal),
+    defaultOutputRules: new DefaultOutputRules()
+  } as ProcessorInitOptions


I would prefer this to be in a .tests sourcefile because having this here means we have redundant JSKeyboardInterface and DefaultOutputRules always constructed even though they will never be used at runtime.

Good point. Done.

- remove confusing comment - move default ProcessorInitOptions to tests folder - add jsdoc comments to ProcessorInitOptions Test-bot: skip

…b/ProcessorInitOptions

mcdurdin · 2026-01-20T21:13:08Z

+ */
+export interface ProcessorInitOptions {
+  /**
+   * The base layout identifier for the keyboard.


What code space is this? It's not BCP-47? Is it just us at this point?

mcdurdin · 2026-01-20T21:15:36Z


      // Now for the real test!
-      let processor = new JSKeyboardProcessor(device);
+      let processor = new JSKeyboardProcessor(device, DEFAULT_PROCESSOR_INIT_OPTIONS);


Where is DEFAULT_PROCESSOR_INIT_OPTIONS defined? It doesn't seem to be imported in this or defined anywhere in this PR?

Hmm, good question. Seems the file got lost along the way. Makes me wonder how the tests all passed...

Makes me wonder how the tests all passed...

Indeed! Please do check this out because it may indicate other problems -- we don't want tests to pass for the wrong reasons.

The definition of `DEFAULT_PROCESSOR_INIT_OPTIONS` got lost in last PR. Test-bot: skip

mcdurdin

LGTM, but please do check to find out why tests still passed with the missing defaultProcessorInitOptions.ts module.

ermshiperete · 2026-01-22T16:33:18Z

LGTM, but please do check to find out why tests still passed with the missing defaultProcessorInitOptions.ts module.

We have both .ts and .js tests. When we reduced the number of build scripts for web, the part that ran the .js tests got lost. I'll fix it in a follow-up PR.

mcdurdin · 2026-01-25T21:23:19Z

I'll fix it in a follow-up PR.

Please link to it here.

ermshiperete added 3 commits January 16, 2026 17:48

refactor(web): options parameter mandatory in InputProcessor c'tor

0d67393

This makes the `options` parameter in the `InputProcessor` constructor no longer optional. Also removes the `DEFAULT_OPTIONS` from `InputProcessor`. Test-bot: skip

refactor(web): make ProcessorInitOptions.keyboardInterface mandatory

aa5c5ba

This change moves the `ProcessorInitOptions` interface to a separate file and makes `keyboardInterface` a required field. Test-bot: skip

ermshiperete requested a review from mcdurdin January 16, 2026 17:16

github-project-automation Bot moved this to Todo in Keyman Jan 16, 2026

github-project-automation Bot added this to Keyman Jan 16, 2026

keymanapp-test-bot Bot added the epic-web-core label Jan 16, 2026

keymanapp-test-bot Bot changed the title ~~refactor(web): make ProcessorInitOptions.keyboardInterface mandatory~~ refactor(web): make ProcessorInitOptions.keyboardInterface mandatory 🎼 Jan 16, 2026

keymanapp-test-bot Bot added this to the A19S20 milestone Jan 16, 2026

github-actions Bot added web/ refactor labels Jan 16, 2026

ermshiperete changed the title ~~refactor(web): make ProcessorInitOptions.keyboardInterface mandatory 🎼~~ refactor(web): refactor ProcessorInitOptions 📏 🎼 Jan 16, 2026

keyman-server modified the milestones: A19S20, A19S21 Jan 16, 2026

mcdurdin requested changes Jan 19, 2026

View reviewed changes

ermshiperete added 2 commits January 20, 2026 12:10

refactor(web): address code review comments

8ecaae5

- remove confusing comment - move default ProcessorInitOptions to tests folder - add jsdoc comments to ProcessorInitOptions Test-bot: skip

chore(web): Merge branch 'refactor/web/defaultRules' into refactor/we…

ad10627

…b/ProcessorInitOptions

ermshiperete requested a review from mcdurdin January 20, 2026 11:18

Base automatically changed from refactor/web/defaultRules to epic/web-core January 20, 2026 11:22

mcdurdin reviewed Jan 20, 2026

View reviewed changes

ermshiperete requested a review from mcdurdin January 21, 2026 12:05

refactor(web): address code review comments

40ce71a

The definition of `DEFAULT_PROCESSOR_INIT_OPTIONS` got lost in last PR. Test-bot: skip

ermshiperete force-pushed the refactor/web/ProcessorInitOptions branch from 1f1a3ef to 40ce71a Compare January 21, 2026 12:07

mcdurdin approved these changes Jan 22, 2026

View reviewed changes

ermshiperete merged commit 7e972ab into epic/web-core Jan 22, 2026
15 of 16 checks passed

ermshiperete deleted the refactor/web/ProcessorInitOptions branch January 22, 2026 16:34

github-project-automation Bot moved this from Todo to Done in Keyman Jan 22, 2026

		// 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.

Uh oh!

Conversation

ermshiperete commented Jan 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

keymanapp-test-bot Bot commented Jan 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

User Test Results

Test Artifacts

Uh oh!

mcdurdin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mcdurdin left a comment

Choose a reason for hiding this comment

Uh oh!

ermshiperete commented Jan 22, 2026

Uh oh!

Uh oh!

mcdurdin commented Jan 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ermshiperete commented Jan 16, 2026 •

edited

Loading

keymanapp-test-bot Bot commented Jan 16, 2026 •

edited

Loading