fix: Use a narrower type for custom shortcut names and reduce hard-coding.

src/actions/shortcuts.ts

@@ -2,11 +2,12 @@ import { isDarwin } from "../constants";
 import { t } from "../i18n";
 import { SubtypeOf } from "../utility-types";
 import { getShortcutKey } from "../utils";
-import { ActionName } from "./types";
+import { ActionName, CustomActionName } from "./types";
 
 export type ShortcutName =
   | SubtypeOf<
       ActionName,
+      | CustomActionName
       | "toggleTheme"
       | "loadScene"
       | "clearCanvas"
@@ -40,6 +41,15 @@ export type ShortcutName =
   | "saveScene"
   | "imageExport";
 
+export const registerCustomShortcuts = (
+  shortcuts: Record<CustomActionName, string[]>,
+) => {
+  for (const key in shortcuts) {
+    const shortcut = key as CustomActionName;
+    shortcutMap[shortcut] = shortcuts[shortcut];
+  }
+};
+
 const shortcutMap: Record<ShortcutName, string[]> = {
   toggleTheme: [getShortcutKey("Shift+Alt+D")],
   saveScene: [getShortcutKey("CtrlOrCmd+S")],
@@ -85,23 +95,8 @@ const shortcutMap: Record<ShortcutName, string[]> = {
   toggleElementLock: [getShortcutKey("CtrlOrCmd+Shift+L")],
 };
 
-export type CustomShortcutName = string;
-
-let customShortcutMap: Record<CustomShortcutName, string[]> = {};
-
-export const registerCustomShortcuts = (
-  shortcuts: Record<CustomShortcutName, string[]>,
-) => {
-  customShortcutMap = { ...customShortcutMap, ...shortcuts };
-};
-
-export const getShortcutFromShortcutName = (
-  name: ShortcutName | CustomShortcutName,
-) => {
-  const shortcuts =
-    name in customShortcutMap
-      ? customShortcutMap[name as CustomShortcutName]
-      : shortcutMap[name as ShortcutName];
+export const getShortcutFromShortcutName = (name: ShortcutName) => {
+  const shortcuts = shortcutMap[name];
   // if multiple shortcuts available, take the first one
   return shortcuts && shortcuts.length > 0 ? shortcuts[0] : "";
 };

src/actions/types.ts

@@ -44,8 +44,11 @@ export type ActionPredicateFn = (
 export type UpdaterFn = (res: ActionResult) => void;
 export type ActionFilterFn = (action: Action) => void;
 
+export const makeCustomActionName = (name: string) =>
+  `custom.${name}` as CustomActionName;
+export type CustomActionName = `custom.${string}`;
 export type ActionName =
-  | `custom.${string}`
+  | CustomActionName
   | "copy"
   | "cut"
   | "paste"

src/components/Subtypes.tsx

@@ -1,6 +1,6 @@
 import { getShortcutKey, updateActiveTool } from "../utils";
 import { t } from "../i18n";
-import { Action } from "../actions/types";
+import { Action, makeCustomActionName } from "../actions/types";
 import clsx from "clsx";
 import {
   Subtype,
@@ -29,7 +29,7 @@ export const SubtypeButton = (
   const keyTest: Action["keyTest"] =
     key !== undefined ? (event) => event.code === `Key${key}` : undefined;
   const subtypeAction: Action = {
-    name: `custom.${subtype}`,
+    name: makeCustomActionName(subtype),
     trackEvent: false,
     predicate: (...rest) => rest[4]?.subtype === subtype,
     perform: (elements, appState) => {
@@ -159,7 +159,7 @@ export const SubtypeToggles = () => {
       >
         {getSubtypeNames().map((subtype) =>
           am.renderAction(
-            `custom.${subtype}`,
+            makeCustomActionName(subtype),
             hasAlwaysEnabledActions(subtype) ? { onContextMenu } : {},
           ),
         )}

src/element/subtypes/index.ts

@@ -5,11 +5,14 @@ import { getSelectedElements } from "../../scene";
 import { AppState, ExcalidrawImperativeAPI } from "../../types";
 import { registerAuxLangData } from "../../i18n";
 
-import { Action, ActionName, ActionPredicateFn } from "../../actions/types";
 import {
-  CustomShortcutName,
-  registerCustomShortcuts,
-} from "../../actions/shortcuts";
+  Action,
+  ActionName,
+  ActionPredicateFn,
+  CustomActionName,
+  makeCustomActionName,
+} from "../../actions/types";
+import { registerCustomShortcuts } from "../../actions/shortcuts";
 import { register } from "../../actions/register";
 import { hasBoundTextElement, isTextElement } from "../typeChecks";
 import {
@@ -44,7 +47,7 @@ export type SubtypeRecord = Readonly<{
   parents: readonly ExcalidrawElement["type"][];
   actionNames?: readonly SubtypeActionName[];
   disabledNames?: readonly DisabledActionName[];
-  shortcutMap?: Record<CustomShortcutName, string[]>;
+  shortcutMap?: Record<string, string[]>;
   alwaysEnabledNames?: readonly SubtypeActionName[];
 }>;
 
@@ -373,8 +376,8 @@ export const prepareSubtype = (
       ...subtypeActionMap,
       {
         subtype,
-        actions: record.actionNames.map(
-          (actionName) => `custom.${actionName}` as ActionName,
+        actions: record.actionNames.map((actionName) =>
+          makeCustomActionName(actionName),
         ),
       },
     ];
@@ -390,14 +393,19 @@ export const prepareSubtype = (
       ...alwaysEnabledMap,
       {
         subtype,
-        actions: record.alwaysEnabledNames.map(
-          (actionName) => `custom.${actionName}` as ActionName,
+        actions: record.alwaysEnabledNames.map((actionName) =>
+          makeCustomActionName(actionName),
         ),
       },
     ];
   }
-  if (record.shortcutMap) {
-    registerCustomShortcuts(record.shortcutMap);
+  const customShortcutMap = record.shortcutMap;
+  if (customShortcutMap) {
+    const shortcutMap: Record<CustomActionName, string[]> = {};
+    for (const key in customShortcutMap) {
+      shortcutMap[makeCustomActionName(key)] = customShortcutMap[key];
+    }
+    registerCustomShortcuts(shortcutMap);
   }
 
   // Prepare the subtype

src/tests/customActions.test.tsx

@@ -4,11 +4,16 @@ import { API } from "./helpers/api";
 import { render } from "./test-utils";
 import { Excalidraw } from "../packages/excalidraw/index";
 import {
-  CustomShortcutName,
   getShortcutFromShortcutName,
   registerCustomShortcuts,
 } from "../actions/shortcuts";
-import { Action, ActionPredicateFn, ActionResult } from "../actions/types";
+import {
+  Action,
+  ActionPredicateFn,
+  ActionResult,
+  CustomActionName,
+  makeCustomActionName,
+} from "../actions/types";
 import {
   actionChangeFontFamily,
   actionChangeFontSize,
@@ -19,11 +24,14 @@ const { h } = window;
 
 describe("regression tests", () => {
   it("should retrieve custom shortcuts", () => {
-    const shortcuts: Record<CustomShortcutName, string[]> = {
-      test: [getShortcutKey("CtrlOrCmd+1"), getShortcutKey("CtrlOrCmd+2")],
-    };
+    const shortcutName = makeCustomActionName("test");
+    const shortcuts: Record<CustomActionName, string[]> = {};
+    shortcuts[shortcutName] = [
+      getShortcutKey("CtrlOrCmd+1"),
+      getShortcutKey("CtrlOrCmd+2"),
+    ];
     registerCustomShortcuts(shortcuts);
-    expect(getShortcutFromShortcutName("test")).toBe("Ctrl+1");
+    expect(getShortcutFromShortcutName(shortcutName)).toBe("Ctrl+1");
   });
 
   it("should apply universal action predicates", async () => {

src/tests/subtypes.test.tsx

@@ -32,7 +32,7 @@ import { getFontString, getShortcutKey } from "../utils";
 import * as textElementUtils from "../element/textElement";
 import { isTextElement } from "../element";
 import { mutateElement, newElementWith } from "../element/mutateElement";
-import { Action, ActionName } from "../actions/types";
+import { Action, ActionName, makeCustomActionName } from "../actions/types";
 import { AppState } from "../types";
 import { getShortcutFromShortcutName } from "../actions/shortcuts";
 import { actionChangeSloppiness } from "../actions";
@@ -81,7 +81,7 @@ const test1: SubtypeRecord = {
 };
 
 const testAction: Action = {
-  name: `custom.${TEST_ACTION}`,
+  name: makeCustomActionName(TEST_ACTION),
   trackEvent: false,
   perform: (elements, appState) => {
     return {
@@ -338,7 +338,9 @@ describe("subtypes", () => {
     expect(test3Methods?.clean).toBeUndefined();
   });
   it("should register custom shortcuts", async () => {
-    expect(getShortcutFromShortcutName("testShortcut")).toBe("Shift+T");
+    expect(
+      getShortcutFromShortcutName(makeCustomActionName("testShortcut")),
+    ).toBe("Shift+T");
   });
   it("should correctly validate", async () => {
     test1.parents.forEach((p) => {