fix(fields): Make SKIP_SETUP a Symbol; remove Sentinel class (#6919)

We introduced the SKIP_SETUP sentinel value when converting
Field and its subclasses to ES6 class syntax, because super
must be called before any other code in a subclass
constructor, breaking the previous mechanism where subclasses
would set some properties before calling their superclass
constructor.

SKIP_SETUP was a singleton value of class Sentinel.

Recently, in PR #6639 @btw17 introduced the isSentinel type
predicate to improve the typing of Field.  Unfortunately, there
were some aspects of this change that were not very elegant:

- isSentinel was declared as a static method on Field (rather
  than on Sentinel itself).
- It only checks against the specific value SKIP_SETUP,
  rather than checking if the argument is instanceof Sentinel
  (though perhaps this is for efficiency?)

Additionally - as a result of the migration from ES6 to TS, and
predating PR #6639 - The signature for the Field constructor's
first argument was typed T|Sentinel, with subclass constructors
generally being <some type(s)>|Sentinel.

This creates a small problem when attempting to port Fields from
core to plugins, because Sentinel is not reexported by
core/utils.ts (and therefore not from core/blockly.ts either).

The latter problem could be solved simply by reexporting Sentinel,
or by having plugin constructors not accept SKIP_SETUP (though
this potentially makes them more difficult to subclass), but
neither is particularly desirable.

Instead, this PR proposes that we:

- Make Field.SKIP_SETUP a (unique) Symbol.
- Change the value argument to the Field constructor to accept
  T|typeof Field.SKIP_SETUP - where typeof Field.SKIP_SETUP is
  (like a literal type) a type that accepts just the single
  value SKIP_SETUP.
- Remove the Sentinel class and core/utils/sentinel.ts.

Not treating this as a breaking change:

- Removes Field.isSentinel - though this addition has not yet
  been published, so it can only break our own as-yet-unreleased
  code in samples.

- Changes the type of Field.SKIP_SETUP and the first argument
  of the Field constructor from Sentinel to typeof SKIP_SETUP
  (a unique Symbol) - but given that Sentinel has never been
  exported this should not break any actual external code.
This commit is contained in:
Christopher Allen
2023-03-23 20:30:48 +00:00
committed by GitHub
parent 01ecf547f4
commit faa95d022d
13 changed files with 36 additions and 86 deletions

View File

@@ -24,7 +24,6 @@ import type {Menu} from './menu.js';
import type {MenuItem} from './menuitem.js';
import {Msg} from './msg.js';
import * as parsing from './utils/parsing.js';
import type {Sentinel} from './utils/sentinel.js';
import {Size} from './utils/size.js';
import {VariableModel} from './variable_model.js';
import * as Variables from './variables.js';
@@ -76,9 +75,9 @@ export class FieldVariable extends FieldDropdown {
* for a list of properties this parameter supports.
*/
constructor(
varName: string|null|Sentinel, validator?: FieldVariableValidator,
variableTypes?: string[], defaultType?: string,
config?: FieldVariableConfig) {
varName: string|null|typeof Field.SKIP_SETUP,
validator?: FieldVariableValidator, variableTypes?: string[],
defaultType?: string, config?: FieldVariableConfig) {
super(Field.SKIP_SETUP);
/**
@@ -97,9 +96,7 @@ export class FieldVariable extends FieldDropdown {
/** The size of the area rendered by the field. */
this.size_ = new Size(0, 0);
if (varName === Field.SKIP_SETUP) {
return;
}
if (varName === Field.SKIP_SETUP) return;
if (config) {
this.configure_(config);