refactor(web): clarify function scope and update some function comments

[ermshiperete]

Build-bot: skip build:web
Test-bot: skip

[keymanapp-test-bot]

User Test Results

Test specification and instructions

User tests are not required

Test Artifacts

• Web (no artifacts on BuildLevel "build")
  • KeymanWeb Test Home - build : ✅ all tests passed

[mcdurdin]

Good catch. However, if !keyboardId, we always (line 333) return quickly anyway:

      return {
          keyboard: Promise.resolve(null),
          metadata: null
      }

So it has no impact because it was never used...

It'd be better to move the parameter nullish check to the top of the function and simplify the logic:

  protected prepareKeyboardForActivation(
    keyboardId: string,
    languageCode?: string
  ): {keyboard: Promise<Keyboard>, metadata: KeyboardStub} {

    if(!keyboardId) {
      return {
        keyboard: Promise.resolve(null),
        metadata: null
      }
    }

    // Set default language code
    languageCode ||= '';

    // Check that the saved keyboard is currently registered
    const requestedStub: KeyboardStub = this.keyboardCache.getStub(keyboardId, languageCode);

    if(!requestedStub) {
      const availableStubList = this.keyboardCache.getStubList().map(stub => `${stub.KI}@${stub.KLC}`);
      throw new Error(`No matching stub has been registered for keyboard ${keyboardId}.  Available stubs: ${JSON.stringify(availableStubList)}`);
    }

    // Check if current keyboard matches requested keyboard, but not (necessarily) stub
    if (keyboardId === this.activeKeyboard?.metadata?.id) {
      const {keyboard} = this.activeKeyboard;
      // In this case, the keyboard is loaded; just update the stub.

      return {
        keyboard: Promise.resolve(keyboard),
        metadata: requestedStub
      };
    }

Secondary question, why would we ever call prepareKeyboardForActivation with a nullish keyboardId? That smells buggy in itself.