fix(plugin-subcommands-advanced): sort registered subcommands by name

.changeset/subcommands-advanced-sort-order.md

@@ -0,0 +1,5 @@
+---
+"@wolfstar/plugin-subcommands-advanced": patch
+---
+
+Register modular subcommands (and groups) sorted by name instead of in the order the stores loaded them. The load order is not stable between starts, so the same pieces could produce differently ordered options, and `@wolfstar/http-framework` compares deployed commands by position, which updated every parent with modular subcommands on each start. The first start after upgrading may update those commands once; the order is stable after that. Subcommands the parent declares itself keep their place before the registered ones.

packages/plugin-subcommands-advanced/src/lib/utils/functions.ts

@@ -19,6 +19,22 @@ export const subCommandsGroupRegistry: Collection<
   Collection<string, Collection<string, SubcommandMappingCollection>>
 > = new Collection();
 
+/**
+ * Entries of a registry sorted by key, without mutating it.
+ *
+ * Registries fill in the order the stores load the pieces, which is not stable between
+ * starts, while Discord keeps option order and `http-framework` compares it by position.
+ * Keys are compared by code unit rather than with `localeCompare`, so the result does not
+ * depend on the machine's locale either.
+ *
+ * @internal
+ */
+export function sortedRegistryEntries<Value>(
+  registry: ReadonlyMap<string, Value>,
+): [string, Value][] {
+  return [...registry].toSorted(([a], [b]) => (a < b ? -1 : a > b ? 1 : 0));
+}
+
 interface DeferredSubcommand {
   parentCommandName: string;
   groupName?: string;

packages/plugin-subcommands-advanced/src/lib/utils/hooks.ts

@@ -1,6 +1,10 @@
 import type { SlashCommandBuilder } from "@discordjs/builders";
 import type { Command } from "@wolfstar/http-framework";
-import { subCommandsGroupRegistry, subCommandsRegistry } from "./functions.js";
+import {
+  sortedRegistryEntries,
+  subCommandsGroupRegistry,
+  subCommandsRegistry,
+} from "./functions.js";
 
 /**
  * Optional hooks for parent commands that register chat-input commands imperatively
@@ -23,7 +27,7 @@ export const RegisterSubcommandsHooks = {
 
     if (!context) return;
 
-    for (const { slashCommand } of subcommands.values()) {
+    for (const [, { slashCommand }] of sortedRegistryEntries(subcommands)) {
       context.addSubcommand(slashCommand);
     }
   },
@@ -42,7 +46,7 @@ export const RegisterSubcommandsHooks = {
 
     if (!context) return;
 
-    for (const [groupName, commands] of groups) {
+    for (const [groupName, commands] of sortedRegistryEntries(groups)) {
       const group = context.options.find((option) => {
         const data =
           "toJSON" in option && typeof option.toJSON === "function" ? option.toJSON() : null;
@@ -51,7 +55,7 @@ export const RegisterSubcommandsHooks = {
 
       if (!group?.addSubcommand) continue;
 
-      for (const { slashCommand } of commands.values()) {
+      for (const [, { slashCommand }] of sortedRegistryEntries(commands)) {
         group.addSubcommand(slashCommand);
       }
     }

packages/plugin-subcommands-advanced/src/lib/utils/wiring.ts

@@ -7,6 +7,7 @@ import {
 import {
   clearDeferredSubcommands,
   resolveDeferredSubcommands,
+  sortedRegistryEntries,
   subCommandsGroupRegistry,
   subCommandsRegistry,
 } from "./functions.js";
@@ -67,16 +68,16 @@ export function wireParentSubcommands(parent: Command): void {
 
   const direct = subCommandsRegistry.get(parentName);
   if (direct) {
-    for (const entry of direct.values()) {
+    for (const [, entry] of sortedRegistryEntries(direct)) {
       installDelegate(ctor, parentName, entry);
       resolver.addSubcommand(entry.slashCommand, entry.methodName);
     }
   }
 
   const groups = subCommandsGroupRegistry.get(parentName);
   if (groups) {
-    for (const [groupName, commands] of groups) {
-      for (const entry of commands.values()) {
+    for (const [groupName, commands] of sortedRegistryEntries(groups)) {
+      for (const [, entry] of sortedRegistryEntries(commands)) {
         installDelegate(ctor, parentName, entry);
         resolver.addSubcommand(entry.slashCommand, entry.methodName, groupName);
       }

packages/plugin-subcommands-advanced/tests/subcommands-advanced.test.ts

@@ -1,5 +1,7 @@
+import { SlashCommandBuilder } from "@discordjs/builders";
 import { ApplicationCommandOptionType, InteractionResponseType } from "discord-api-types/v10";
 import {
+  applicationCommandRegistry,
   container,
   RegisterCommand,
   RegisterSubcommand,
@@ -15,6 +17,7 @@ import {
   Command,
   RegisterAsSubcommand,
   RegisterAsSubcommandGroup,
+  RegisterSubcommandsHooks,
   Subcommand,
   subCommandsRegistry,
   wireParentSubcommands,
@@ -40,6 +43,8 @@ async function loadParentAndChildren(
   }
 }
 
+const optionNames = (options: { name: string }[]) => options.map((option) => option.name);
+
 describe("plugin-subcommands-advanced", () => {
   afterEach(async () => {
     clearSubcommandRegistries();
@@ -275,4 +280,92 @@ describe("plugin-subcommands-advanced", () => {
       data: { content: "echo" },
     });
   });
+
+  describe("subcommand order", () => {
+    const childNames = ["unblock", "block", "history"];
+
+    function defineChildren(groupName?: string) {
+      return childNames.map((name) => {
+        const decorate = groupName
+          ? RegisterAsSubcommandGroup("reports", groupName, (builder) =>
+              builder.setName(name).setDescription(name),
+            )
+          : RegisterAsSubcommand("reports", (builder) =>
+              builder.setName(name).setDescription(name),
+            );
+
+        @decorate
+        class Child extends Command {
+          public override chatInputRun(interaction: Command.ChatInputInteraction) {
+            return interaction.reply({ content: name });
+          }
+        }
+
+        return Child;
+      });
+    }
+
+    function wireInOrder(order: number[], grouped: boolean) {
+      createTestHarness({ subcommandsAdvanced: { nameCommandsAutogenerated: true } });
+
+      @RegisterCommand((builder) => {
+        builder.setName("reports").setDescription("Reports");
+        if (grouped) builder.addSubcommandGroup((group) => group.setName("g").setDescription("g"));
+        return builder.addSubcommand((sub) => sub.setName("native").setDescription("native"));
+      })
+      class ReportsCommand extends Subcommand {}
+
+      const classes = defineChildren(grouped ? "g" : undefined);
+      for (const index of order) makeCommand(classes[index]!);
+      const parent = makeCommand(ReportsCommand);
+      wireParentSubcommands(parent);
+
+      const options = applicationCommandRegistry.get(ReportsCommand)!.chatInput!.toJSON().options!;
+      return { parent, options };
+    }
+
+    const findGroup = (options: ReturnType<typeof wireInOrder>["options"]) =>
+      options.find(
+        (option) => option.type === ApplicationCommandOptionType.SubcommandGroup,
+      ) as unknown as { options: { name: string }[] };
+
+    it("wires direct subcommands in the same order whatever order they loaded in", () => {
+      const forward = wireInOrder([0, 1, 2], false).options;
+      clearSubcommandRegistries();
+      const backward = wireInOrder([2, 1, 0], false).options;
+
+      expect(optionNames(forward)).toEqual(["native", "block", "history", "unblock"]);
+      expect(backward).toEqual(forward);
+    });
+
+    it("wires grouped subcommands in the same order whatever order they loaded in", () => {
+      const forward = wireInOrder([0, 1, 2], true).options;
+      clearSubcommandRegistries();
+      const backward = wireInOrder([2, 1, 0], true).options;
+
+      expect(optionNames(findGroup(forward).options)).toEqual(["block", "history", "unblock"]);
+      expect(backward).toEqual(forward);
+    });
+
+    it("attaches subcommands and groups to a builder sorted by name", () => {
+      const direct = wireInOrder([0, 1, 2], false);
+      const directBuilder = new SlashCommandBuilder().setName("reports").setDescription("Reports");
+      RegisterSubcommandsHooks.subcommands(direct.parent, directBuilder);
+      expect(optionNames(directBuilder.toJSON().options as { name: string }[])).toEqual([
+        "block",
+        "history",
+        "unblock",
+      ]);
+
+      clearSubcommandRegistries();
+      const grouped = wireInOrder([2, 0, 1], true);
+      const groupBuilder = new SlashCommandBuilder()
+        .setName("reports")
+        .setDescription("Reports")
+        .addSubcommandGroup((group) => group.setName("g").setDescription("g"));
+      RegisterSubcommandsHooks.groups(grouped.parent, groupBuilder);
+      const [group] = groupBuilder.toJSON().options as { options: { name: string }[] }[];
+      expect(optionNames(group!.options)).toEqual(["block", "history", "unblock"]);
+    });
+  });
 });