From 67cc65ae7115d3b7e7bc5bad94445f665a52948e Mon Sep 17 00:00:00 2001 From: RedStar Date: Sat, 17 Oct 2026 21:25:44 +0200 Subject: [PATCH] fix(plugin-subcommands-advanced): sort registered subcommands by name Registered subcommands followed store load order, which changes between starts. http-framework then updated the parent command on every restart. Iterate them sorted by name when wiring and in the register hooks. --- .changeset/subcommands-advanced-sort-order.md | 5 + .../src/lib/utils/functions.ts | 16 ++++ .../src/lib/utils/hooks.ts | 12 ++- .../src/lib/utils/wiring.ts | 7 +- .../tests/subcommands-advanced.test.ts | 93 +++++++++++++++++++ 5 files changed, 126 insertions(+), 7 deletions(-) create mode 100644 .changeset/subcommands-advanced-sort-order.md diff --git a/.changeset/subcommands-advanced-sort-order.md b/.changeset/subcommands-advanced-sort-order.md new file mode 100644 index 00000000..90a62982 --- /dev/null +++ b/.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. diff --git a/packages/plugin-subcommands-advanced/src/lib/utils/functions.ts b/packages/plugin-subcommands-advanced/src/lib/utils/functions.ts index 7053332b..e8e21fe6 100644 --- a/packages/plugin-subcommands-advanced/src/lib/utils/functions.ts +++ b/packages/plugin-subcommands-advanced/src/lib/utils/functions.ts @@ -19,6 +19,22 @@ export const subCommandsGroupRegistry: Collection< Collection> > = 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( + registry: ReadonlyMap, +): [string, Value][] { + return [...registry].toSorted(([a], [b]) => (a < b ? -1 : a > b ? 1 : 0)); +} + interface DeferredSubcommand { parentCommandName: string; groupName?: string; diff --git a/packages/plugin-subcommands-advanced/src/lib/utils/hooks.ts b/packages/plugin-subcommands-advanced/src/lib/utils/hooks.ts index e86d7b9e..dc0aa402 100644 --- a/packages/plugin-subcommands-advanced/src/lib/utils/hooks.ts +++ b/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); } } diff --git a/packages/plugin-subcommands-advanced/src/lib/utils/wiring.ts b/packages/plugin-subcommands-advanced/src/lib/utils/wiring.ts index 8c1dfa00..316dfe5e 100644 --- a/packages/plugin-subcommands-advanced/src/lib/utils/wiring.ts +++ b/packages/plugin-subcommands-advanced/src/lib/utils/wiring.ts @@ -7,6 +7,7 @@ import { import { clearDeferredSubcommands, resolveDeferredSubcommands, + sortedRegistryEntries, subCommandsGroupRegistry, subCommandsRegistry, } from "./functions.js"; @@ -67,7 +68,7 @@ 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); } @@ -75,8 +76,8 @@ export function wireParentSubcommands(parent: Command): void { 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); } diff --git a/packages/plugin-subcommands-advanced/tests/subcommands-advanced.test.ts b/packages/plugin-subcommands-advanced/tests/subcommands-advanced.test.ts index 5a608619..fcd9d75f 100644 --- a/packages/plugin-subcommands-advanced/tests/subcommands-advanced.test.ts +++ b/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["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"]); + }); + }); });