fix: updated field_dropdown to properly infer its Field type with TS 5.3.3

[btw17]

The basics

• I validated my changes

The details

Resolves

Fixes #7715

Proposed Changes

Update FieldDropdown's type information for doClassValidation to match the template in Field so it properly infers FieldDropdown as Field<string> instead of Field<string|undefined>.

Reason for Changes

To enable Blockly to update to TypeScript 5.3.3 and beyond.

Test Coverage

N/A

Documentation

N/A

Additional Information

None

[conventional-commit-lint-gcf]

🤖 I detect that the PR title and the commit message differ and there's only one commit. To use the PR title for the commit history, you can use Github's automerge feature with squashing, or use automerge label. Good luck human!

-- conventional-commit-lint bot
https://conventionalcommits.org/

[rachel-fenichel]

Is this a bad diff, or are there three lines in a row declaring the same function? And if it's the latter, why is that necessary and allowed?

[btw17]

This is accurate. This is actually how we have it defined in field.ts, too:
https://github.com/google/blockly/blob/master/core/field.ts#L1183

What this does is allow inferring type info based on what you're supplying. Taking Field's doClassValidation as an example, implementation-wise, undefined means to take new value as is instead of transforming it in some way. What happens is if you provide a newValue as a non-undefined value, it infers the first definition that matches.

  // Providing a non-undefined value allows returning `undefined` (which means to take that value as is)
  protected doClassValidation_(newValue: T): T | null | undefined;
  // Providing an undefined or possibly undefined value means you can't return `undefined` since
  // that would mean take the default, though default could be `undefined` which is not valid to set.
  protected doClassValidation_(newValue?: AnyDuringMigration): T | null;
  // This accepts all the implementations above and we handle the cases in code through this.
  protected doClassValidation_(
    newValue?: T | AnyDuringMigration,
  ): T | null | undefined {
    if (newValue === null || newValue === undefined) {
      return null;
    }

    return newValue as T;
  }

[btw17]

Just noticed a nitpick - even thoug the base class has | undefined, this class has never expected that and it's actually not needed here. (I confirmed locally.) Updating the slight tweak.

[maribethb]

class validators are allowed to return undefined: https://developers.google.com/blockly/guides/create-custom-blocks/fields/validators#return_values

I think we need to keep the | undefined to allow subclasses of Dropdown to continue returning undefined

[btw17]

Sounds good! I'll make the update.

[btw17]

Done!