From 9eeb4fea0b4cdc920228b23fc7743ab1ffdb9b0f Mon Sep 17 00:00:00 2001 From: Christopher Allen Date: Wed, 29 Jun 2022 12:14:34 +0100 Subject: [PATCH] fix: Correct enum formatting, use merged `namespace`s for types that are class static members (#6246) * fix: formatting of enum KeyCodes * fix: Use merged namespace for ContextMenuRegistry static types - Create a namespace to be merged with the ContextMenuRegistry class containing the types that were formerly declared as static properties on that class. - Use type aliases to export them individually as well, for compatibility with the changes made by MigranTS (and/or @gonfunko) to how other modules in core/ now import these types. - Update renamings.json5 to reflect the availability of the direct exports for modules that import this module directly (though they are not available to, and will not be used by, code that imports only via blockly.js/blockly.ts.) * fix: Use merged namespace for Input.Align - Create a merged namespace for the Input.Align enum. - Use type/const aliases to export it as Input too. - Update renamings.json5 to reflect the availability of the direct export. * fix: Use merged namespace for Names.NameType - Create a merged namespace for the Names.NameType enum. - Use type/const aliases to export it as NameType too. - Update renamings.json5 to reflect the availability of the direct export. (This ought to have happened in an earlier version as it was already available by both routes.) * chore: Fix minor issues for PR #6246 - Use `Align` instead of `Input.Align` where possible. * fix(build): Suppress irrelevant JSC_UNUSED_LOCAL_ASSIGNMENT errors tsc generates code for merged namespaces that looks like: (function (ClassName) { let EnumName; (function (EnumName) { EnumName[EnumNameAlign["v1"] = 0] = "v1"; // etc. })(EnumName = ClassName.EnumName || (ClassName.EnumName = {})); })(ClassName || (ClassName = {})); and Closure Compiler complains about the fact that the EnumName let binding is initialised but never used. (It exists so that any other code that was in the namespace could see the enum.) Suppress this message, since it is not actionable and lint and/or tsc should tell us if we have actual unused variables in our .ts files. --- core/contextmenu_registry.ts | 105 ++++++++----- core/input.ts | 31 ++-- core/names.ts | 40 ++--- core/utils/keycodes.ts | 244 +++++++++++++----------------- scripts/gulpfiles/build_tasks.js | 3 +- scripts/migration/renamings.json5 | 52 ++++++- 6 files changed, 262 insertions(+), 213 deletions(-) diff --git a/core/contextmenu_registry.ts b/core/contextmenu_registry.ts index 7bbf5c3c3..a3fa6aba0 100644 --- a/core/contextmenu_registry.ts +++ b/core/contextmenu_registry.ts @@ -21,11 +21,6 @@ import {BlockSvg} from './block_svg.js'; import {WorkspaceSvg} from './workspace_svg.js'; -enum ScopeType { - BLOCK = 'block', - WORKSPACE = 'workspace', -} - /** * Class for the registry of context menu items. This is intended to be a * singleton. You should not create a new instance, and only access this class @@ -33,12 +28,6 @@ enum ScopeType { * @alias Blockly.ContextMenuRegistry */ export class ContextMenuRegistry { - /** - * Where this menu item should be rendered. If the menu item should be - * rendered in multiple scopes, e.g. on both a block and a workspace, it - * should be registered for each scope. - */ - static ScopeType = ScopeType; static registry: ContextMenuRegistry; // TODO(b/109816955): remove '!', see go/strict-prop-init-fix. private registry_!: {[key: string]: RegistryItem}; @@ -125,33 +114,71 @@ export class ContextMenuRegistry { return menuOptions; } } -export interface Scope { - block: BlockSvg|undefined; - workspace: WorkspaceSvg|undefined; -} -export interface RegistryItem { - callback: (p1: Scope) => AnyDuringMigration; - scopeType: ScopeType; - displayText: ((p1: Scope) => string)|string; - preconditionFn: (p1: Scope) => string; - weight: number; - id: string; -} -export interface ContextMenuOption { - text: string; - enabled: boolean; - callback: (p1: Scope) => AnyDuringMigration; - scope: Scope; - weight: number; -} -export interface LegacyContextMenuOption { - text: string; - enabled: boolean; - callback: (p1: Scope) => AnyDuringMigration; + +export namespace ContextMenuRegistry { + /** + * Where this menu item should be rendered. If the menu item should be + * rendered in multiple scopes, e.g. on both a block and a workspace, it + * should be registered for each scope. + */ + export enum ScopeType { + BLOCK = 'block', + WORKSPACE = 'workspace', + } + + /** + * The actual workspace/block where the menu is being rendered. This is passed + * to callback and displayText functions that depend on this information. + */ + export interface Scope { + block: BlockSvg|undefined; + workspace: WorkspaceSvg|undefined; + } + + /** + * A menu item as entered in the registry. + */ + export interface RegistryItem { + callback: (p1: Scope) => AnyDuringMigration; + scopeType: ScopeType; + displayText: ((p1: Scope) => string)|string; + preconditionFn: (p1: Scope) => string; + weight: number; + id: string; + } + + /** + * A menu item as presented to contextmenu.js. + */ + export interface ContextMenuOption { + text: string; + enabled: boolean; + callback: (p1: Scope) => AnyDuringMigration; + scope: Scope; + weight: number; + } + + /** + * A subset of ContextMenuOption corresponding to what was publicly + * documented. ContextMenuOption should be preferred for new code. + */ + export interface LegacyContextMenuOption { + text: string; + enabled: boolean; + callback: (p1: Scope) => AnyDuringMigration; + } + + /** + * Singleton instance of this class. All interactions with this class should + * be done on this object. + */ + ContextMenuRegistry.registry = new ContextMenuRegistry(); } -/** - * Singleton instance of this class. All interactions with this class should be - * done on this object. - */ -ContextMenuRegistry.registry = new ContextMenuRegistry(); +export type ScopeType = ContextMenuRegistry.ScopeType; +export const ScopeType = ContextMenuRegistry.ScopeType; +export type Scope = ContextMenuRegistry.Scope; +export type RegistryItem = ContextMenuRegistry.RegistryItem; +export type ContextMenuOption = ContextMenuRegistry.ContextMenuOption; +export type LegacyContextMenuOption = + ContextMenuRegistry.LegacyContextMenuOption; diff --git a/core/input.ts b/core/input.ts index 07ea37a69..9a297c7b0 100644 --- a/core/input.ts +++ b/core/input.ts @@ -37,10 +37,9 @@ import {RenderedConnection} from './rendered_connection.js'; * @alias Blockly.Input */ export class Input { - static Align: AnyDuringMigration; private sourceBlock_: Block; fieldRow: Field[] = []; - align: number; + align: Align; /** Is the input visible? */ private visible_ = true; @@ -238,11 +237,11 @@ export class Input { /** * Change the alignment of the connection's field(s). - * @param align One of the values of Align In RTL mode directions are - * reversed, and Align.RIGHT aligns to the left. + * @param align One of the values of Align. In RTL mode directions + * are reversed, and Align.RIGHT aligns to the left. * @return The input being modified (to allow chaining). */ - setAlign(align: number): Input { + setAlign(align: Align): Input { this.align = align; if (this.sourceBlock_.rendered) { const sourceBlock = this.sourceBlock_ as BlockSvg; @@ -302,15 +301,17 @@ export class Input { } } -/** - * Enum for alignment of inputs. - * @alias Blockly.Input.Align - */ -export enum Align { - LEFT = -1, - CENTRE, - RIGHT +export namespace Input { + /** + * Enum for alignment of inputs. + * @alias Blockly.Input.Align + */ + export enum Align { + LEFT = -1, + CENTRE = 0, + RIGHT = 1, + } } -// Add Align to Input so that `Blockly.Input.Align` is publicly accessible. -Input.Align = Align; +export type Align = Input.Align; +export const Align = Input.Align; diff --git a/core/names.ts b/core/names.ts index 5b0891481..046a7ee00 100644 --- a/core/names.ts +++ b/core/names.ts @@ -32,8 +32,7 @@ import {Workspace} from './workspace.js'; * @alias Blockly.Names */ export class Names { - static NameType: AnyDuringMigration; - static DEVELOPER_VARIABLE_TYPE: AnyDuringMigration; + static DEVELOPER_VARIABLE_TYPE: NameType; private readonly variablePrefix_: string; private readonly reservedDict_: AnyDuringMigration; private db_: {[key: string]: {[key: string]: string}}; @@ -250,26 +249,27 @@ export class Names { } } -/** - * Enum for the type of a name. Different name types may have different rules - * about collisions. - * When JavaScript (or most other languages) is generated, variable 'foo' and - * procedure 'foo' would collide. However, Blockly has no such problems since - * variable get 'foo' and procedure call 'foo' are unambiguous. - * Therefore, Blockly keeps a separate name type to disambiguate. - * getName('foo', 'VARIABLE') -> 'foo' - * getName('foo', 'PROCEDURE') -> 'foo2' - * @alias Blockly.Names.NameType - */ -export enum NameType { - DEVELOPER_VARIABLE = 'DEVELOPER_VARIABLE', - VARIABLE = 'VARIABLE', - PROCEDURE = 'PROCEDURE' +export namespace Names { + /** + * Enum for the type of a name. Different name types may have different rules + * about collisions. + * When JavaScript (or most other languages) is generated, variable 'foo' and + * procedure 'foo' would collide. However, Blockly has no such problems since + * variable get 'foo' and procedure call 'foo' are unambiguous. + * Therefore, Blockly keeps a separate name type to disambiguate. + * getName('foo', 'VARIABLE') -> 'foo' + * getName('foo', 'PROCEDURE') -> 'foo2' + * @alias Blockly.Names.NameType + */ + export enum NameType { + DEVELOPER_VARIABLE = 'DEVELOPER_VARIABLE', + VARIABLE = 'VARIABLE', + PROCEDURE = 'PROCEDURE', + } } -// We have to export NameType here so that it is accessible under the old name -// `Blockly.Names.NameType` -Names.NameType = NameType; +export type NameType = Names.NameType; +export const NameType = Names.NameType; /** * Constant to separate developer variable names from user-defined variable diff --git a/core/utils/keycodes.ts b/core/utils/keycodes.ts index 50c677167..06d8c7b61 100644 --- a/core/utils/keycodes.ts +++ b/core/utils/keycodes.ts @@ -31,161 +31,131 @@ goog.declareModuleId('Blockly.utils.KeyCodes'); * @alias Blockly.utils.KeyCodes */ export enum KeyCodes { - WIN_KEY_FF_LINUX, + WIN_KEY_FF_LINUX = 0, MAC_ENTER = 3, BACKSPACE = 8, - TAB, - NUM_CENTER = 12, - // NUMLOCK on FF/Safari Mac - ENTER, + TAB = 9, + NUM_CENTER = 12, // NUMLOCK on FF/Safari Mac + ENTER = 13, SHIFT = 16, - CTRL, - ALT, - PAUSE, - CAPS_LOCK, + CTRL = 17, + ALT = 18, + PAUSE = 19, + CAPS_LOCK = 20, ESC = 27, SPACE = 32, - PAGE_UP, - // also NUM_NORTH_EAST - PAGE_DOWN, - // also NUM_SOUTH_EAST - END, - // also NUM_SOUTH_WEST - HOME, - // also NUM_NORTH_WEST - LEFT, - // also NUM_WEST - UP, - // also NUM_NORTH - RIGHT, - // also NUM_EAST - DOWN, - // also NUM_SOUTH - PLUS_SIGN = 43, - // NOT numpad plus - PRINT_SCREEN, - INSERT, - // also NUM_INSERT - DELETE, - // also NUM_DELETE + PAGE_UP = 33, // also NUM_NORTH_EAST + PAGE_DOWN = 34, // also NUM_SOUTH_EAST + END = 35, // also NUM_SOUTH_WEST + HOME = 36, // also NUM_NORTH_WEST + LEFT = 37, // also NUM_WEST + UP = 38, // also NUM_NORTH + RIGHT = 39, // also NUM_EAST + DOWN = 40, // also NUM_SOUTH + PLUS_SIGN = 43, // NOT numpad plus + PRINT_SCREEN = 44, + INSERT = 45, // also NUM_INSERT + DELETE = 46, // also NUM_DELETE ZERO = 48, - ONE, - TWO, - THREE, - FOUR, - FIVE, - SIX, - SEVEN, - EIGHT, - NINE, - FF_SEMICOLON = 59, - // Firefox (Gecko) fires this for semicolon instead of 186 - FF_EQUALS = 61, - // Firefox (Gecko) fires this for equals instead of 187 - FF_DASH = 173, - // Firefox (Gecko) fires this for dash instead of 189 + ONE = 49, + TWO = 50, + THREE = 51, + FOUR = 52, + FIVE = 53, + SIX = 54, + SEVEN = 55, + EIGHT = 56, + NINE = 57, + FF_SEMICOLON = 59, // Firefox (Gecko) fires this for semicolon instead of 186 + FF_EQUALS = 61, // Firefox (Gecko) fires this for equals instead of 187 + FF_DASH = 173, // Firefox (Gecko) fires this for dash instead of 189 // Firefox (Gecko) fires this for # on UK keyboards, rather than // Shift+SINGLE_QUOTE. FF_HASH = 163, - QUESTION_MARK = 63, - // needs localization - AT_SIGN, - A, - B, - C, - D, - E, - F, - G, - H, - I, - J, - K, - L, - M, - N, - O, - P, - Q, - R, - S, - T, - U, - V, - W, - X, - Y, - Z, - META, - // WIN_KEY_LEFT - WIN_KEY_RIGHT, - CONTEXT_MENU, + QUESTION_MARK = 63, // needs localization + AT_SIGN = 64, + A = 65, + B = 66, + C = 67, + D = 68, + E = 69, + F = 70, + G = 71, + H = 72, + I = 73, + J = 74, + K = 75, + L = 76, + M = 77, + N = 78, + O = 79, + P = 80, + Q = 81, + R = 82, + S = 83, + T = 84, + U = 85, + V = 86, + W = 87, + X = 88, + Y = 89, + Z = 90, + META = 91, // WIN_KEY_LEFT + WIN_KEY_RIGHT = 92, + CONTEXT_MENU = 93, NUM_ZERO = 96, - NUM_ONE, - NUM_TWO, - NUM_THREE, - NUM_FOUR, - NUM_FIVE, - NUM_SIX, - NUM_SEVEN, - NUM_EIGHT, - NUM_NINE, - NUM_MULTIPLY, - NUM_PLUS, + NUM_ONE = 97, + NUM_TWO = 98, + NUM_THREE = 99, + NUM_FOUR = 100, + NUM_FIVE = 101, + NUM_SIX = 102, + NUM_SEVEN = 103, + NUM_EIGHT = 104, + NUM_NINE = 105, + NUM_MULTIPLY = 106, + NUM_PLUS = 107, NUM_MINUS = 109, - NUM_PERIOD, - NUM_DIVISION, - F1, - F2, - F3, - F4, - F5, - F6, - F7, - F8, - F9, - F10, - F11, - F12, + NUM_PERIOD = 110, + NUM_DIVISION = 111, + F1 = 112, + F2 = 113, + F3 = 114, + F4 = 115, + F5 = 116, + F6 = 117, + F7 = 118, + F8 = 119, + F9 = 120, + F10 = 121, + F11 = 122, + F12 = 123, NUMLOCK = 144, - SCROLL_LOCK, + SCROLL_LOCK = 145, // OS-specific media keys like volume controls and browser controls. FIRST_MEDIA_KEY = 166, LAST_MEDIA_KEY = 183, - SEMICOLON = 186, - // needs localization - DASH = 189, - // needs localization - EQUALS = 187, - // needs localization - COMMA, - // needs localization - PERIOD = 190, - // needs localization - SLASH, - // needs localization - APOSTROPHE, - // needs localization - TILDE = 192, - // needs localization - SINGLE_QUOTE = 222, - // needs localization - OPEN_SQUARE_BRACKET = 219, - // needs localization - BACKSLASH, - // needs localization - CLOSE_SQUARE_BRACKET, - // needs localization + SEMICOLON = 186, // needs localization + DASH = 189, // needs localization + EQUALS = 187, // needs localization + COMMA = 188, // needs localization + PERIOD = 190, // needs localization + SLASH = 191, // needs localization + APOSTROPHE = 192, // needs localization + TILDE = 192, // needs localization + SINGLE_QUOTE = 222, // needs localization + OPEN_SQUARE_BRACKET = 219, // needs localization + BACKSLASH = 220, // needs localization + CLOSE_SQUARE_BRACKET = 221, // needs localization WIN_KEY = 224, - MAC_FF_META = 224, - // Firefox (Gecko) fires this for the meta key instead of 91 - MAC_WK_CMD_LEFT = 91, - // WebKit Left Command key fired, same as META - MAC_WK_CMD_RIGHT = 93, - // WebKit Right Command key fired, different from META + MAC_FF_META = + 224, // Firefox (Gecko) fires this for the meta key instead of 91 + MAC_WK_CMD_LEFT = 91, // WebKit Left Command key fired, same as META + MAC_WK_CMD_RIGHT = 93, // WebKit Right Command key fired, different from META WIN_IME = 229, + // "Reserved for future use". Some programs (e.g. the SlingPlayer 2.4 ActiveX // control) fire this as a hacky way to disable screensavers. VK_NONAME = 252, @@ -195,5 +165,5 @@ export enum KeyCodes { // they're all using Dell Inspiron laptops, so we suspect that this // indicates a hardware/bios problem. // http://en.community.dell.com/support-forums/laptop/f/3518/p/19285957/19523128.aspx - PHANTOM = 255 + PHANTOM = 255, } diff --git a/scripts/gulpfiles/build_tasks.js b/scripts/gulpfiles/build_tasks.js index d949b068d..b02ec0ac9 100644 --- a/scripts/gulpfiles/build_tasks.js +++ b/scripts/gulpfiles/build_tasks.js @@ -212,7 +212,7 @@ var JSCOMP_ERROR = [ 'undefinedVars', 'underscore', 'unknownDefines', - 'unusedLocalVariables', + // 'unusedLocalVariables', // Disabled; see note in JSCOMP_OFF. 'unusedPrivateMembers', 'uselessCode', 'untranspilableFeatures', @@ -251,6 +251,7 @@ var JSCOMP_OFF = [ */ 'checkTypes', 'nonStandardJsDocs', // Due to @internal + 'unusedLocalVariables', // Due to code generated for merged namespaces. /* In order to transition to ES modules, modules will need to import * one another by relative paths. This means that the previous diff --git a/scripts/migration/renamings.json5 b/scripts/migration/renamings.json5 index b1e9ef208..738ebdb3e 100644 --- a/scripts/migration/renamings.json5 +++ b/scripts/migration/renamings.json5 @@ -1341,6 +1341,56 @@ { 'oldName': 'Blockly.utils.global', 'newPath': 'globalThis', - } + }, + { + 'oldName': 'Blockly.ContextMenuRegistry', + exports: { + 'ContextMenuRegistry.ScopeType': { + oldPath: 'Blockly.ContextMenuRegistry.ScopeType', + newExport: 'ScopeType', + newPath: 'Blockly.ContextMenuRegistry.ScopeType', + }, + 'ContextMenuRegistry.Scope': { + oldPath: 'Blockly.ContextMenuRegistry.Scope', + newExport: 'Scope', + newPath: 'Blockly.ContextMenuRegistry.Scope', + }, + 'ContextMenuRegistry.RegistryItem': { + oldPath: 'Blockly.ContextMenuRegistry.RegistryItem', + newExport: 'RegistryItem', + newPath: 'Blockly.ContextMenuRegistry.RegistryItem', + }, + 'ContextMenuRegistry.ContextMenuOption': { + oldPath: 'Blockly.ContextMenuRegistry.ContextMenuOption', + newExport: 'ContextMenuOption', + newPath: 'Blockly.ContextMenuRegistry.ContextMenuOption', + }, + 'ContextMenuRegistry.LegacyContextMenuOption': { + oldPath: 'Blockly.ContextMenuRegistry.LegacyContextMenuOption', + newExport: 'LegacyContextMenuOption', + newPath: 'Blockly.ContextMenuRegistry.LegacyContextMenuOption', + }, + }, + }, + { + 'oldName': 'Blockly.Input', + exports: { + 'Input.Align': { + oldPath: 'Blockly.Input.Align', + newExport: 'Align', + newPath: 'Blockly.Input.Align', + }, + }, + }, + { + 'oldName': 'Blockly.Names', + exports: { + 'Names.NameType': { + oldPath: 'Blockly.Names.NameType', + newExport: 'NameType', + newPath: 'Blockly.Names.NameType', + }, + }, + }, ], }