From bf544cd419c5f17535c0d5af929f7ecd41188320 Mon Sep 17 00:00:00 2001 From: Simon Klee Date: Sat, 31 Oct 2026 11:09:49 +0100 Subject: [PATCH] fix(tui): keep keymap command objects while their data is unchanged (#59294) --- packages/tui/src/context/keymap.tsx | 42 +++++++++++- packages/tui/test/keymap.test.tsx | 103 ++++++++++++++++++++++++++++ 2 files changed, 143 insertions(+), 2 deletions(-) diff --git a/packages/tui/src/context/keymap.tsx b/packages/tui/src/context/keymap.tsx index 521b3aeac86..d6508190600 100644 --- a/packages/tui/src/context/keymap.tsx +++ b/packages/tui/src/context/keymap.tsx @@ -204,6 +204,9 @@ function use(): Keymap { function createLayer(input: () => KeymapLayer) { const value = useValue() const enabled = useInteractivity() + // The keymap rebuilds a re-registered layer when a command's `opencode` object is new, + // so each command keeps one object while its data is unchanged. + const stable = new Map() useBindings(() => { const layer = input() const { commands, bindings, mode, ...options } = layer @@ -213,11 +216,19 @@ function createLayer(input: () => KeymapLayer) { enabled: enabled() ? options.enabled : false, ...(mode === "global" ? {} : { mode: mode ?? MODE.base }), commands: grouped.named.map((command) => { - const { id, description, group, palette, bind, run, ...definition } = command + const previous = stable.get(command.id) + if (previous && sameData(previous, command)) + Object.assign( + previous, + Object.fromEntries(Object.entries(command).filter(([, value]) => typeof value === "function")), + ) + else stable.set(command.id, command) + const kept = stable.get(command.id)! + const { id, description, group, palette, bind, run, ...definition } = kept return { ...definition, name: id, - opencode: command, + opencode: kept, run: (context: CommandContext) => run(value.input(id), context.event), ...(description === undefined ? {} : { desc: description }), ...(group === undefined ? {} : { category: group }), @@ -250,6 +261,33 @@ function createLayer(input: () => KeymapLayer) { }) } +// Compares data fields only; functions are new closures on every run and take the latest by assignment. +function sameData(previous: KeymapCommand, next: KeymapCommand) { + const keys = Object.keys(next) as (keyof KeymapCommand)[] + return ( + keys.length === Object.keys(previous).length && + keys.every((key) => + typeof next[key] === "function" + ? typeof previous[key] === "function" + : Object.hasOwn(previous, key) && sameValue(previous[key], next[key]), + ) + ) +} + +// Layers write `slash` inline, so its objects and arrays compare by their fields. +function sameValue(previous: unknown, next: unknown): boolean { + if (Object.is(previous, next)) return true + if (typeof previous !== "object" || typeof next !== "object" || previous === null || next === null) return false + if (Array.isArray(previous) !== Array.isArray(next)) return false + const before = previous as Record + const after = next as Record + const keys = Object.keys(after) + return ( + keys.length === Object.keys(before).length && + keys.every((key) => Object.hasOwn(before, key) && sameValue(before[key], after[key])) + ) +} + function groupCommands(commands: readonly KeymapCommand[] = []) { return commands.reduce( (result, command) => { diff --git a/packages/tui/test/keymap.test.tsx b/packages/tui/test/keymap.test.tsx index f07a4beebb2..d45055fe008 100644 --- a/packages/tui/test/keymap.test.tsx +++ b/packages/tui/test/keymap.test.tsx @@ -1,6 +1,9 @@ /** @jsxImportSource @opentui/solid */ +import type { KeymapCommand } from "@opencode/plugin/tui/context" +import { useKeymap } from "@opentui/keymap/solid" import { testRender } from "@opentui/solid" import { expect, test } from "bun:test" +import { createSignal } from "solid-js" import { ConfigProvider } from "../src/config" import { Keymap } from "../src/context/keymap" import { createTuiResolvedConfig } from "./fixture/tui-runtime" @@ -100,6 +103,106 @@ test("returns every formatted command shortcut", async () => { } }) +test("re-registered commands keep their opencode object until its data changes", async () => { + const [title, setTitle] = createSignal("Demo") + const [description, setDescription] = createSignal("Runs the demo") + const [version, setVersion] = createSignal(0) + let opencode = () => undefined as KeymapCommand | undefined + let command = () => undefined as KeymapCommand | undefined + + function Harness() { + const keymap = useKeymap() + const commands = Keymap.useCommands() + Keymap.createLayer(() => ({ + commands: [ + { + id: "demo.command", + title: title(), + ...(description() === undefined ? {} : { description: description() }), + palette: true, + suggested: version() >= 0, + run() {}, + }, + ], + })) + opencode = () => + keymap.getCommandEntries({ visibility: "registered" }).find((entry) => entry.command.name === "demo.command") + ?.command.opencode + command = () => commands().find((item) => item.id === "demo.command") + return + } + + const app = await testRender(() => ( + + + + + + )) + try { + const first = opencode() + setVersion(1) + expect(opencode()).toBe(first) + + setTitle("Renamed") + expect(opencode()).not.toBe(first) + expect(command()?.title).toBe("Renamed") + + setDescription(undefined) + expect(command()?.description).toBeUndefined() + } finally { + app.renderer.destroy() + } +}) + +test("re-registered commands keep inline slash objects until their fields change", async () => { + const [aliases, setAliases] = createSignal(["d"]) + const [version, setVersion] = createSignal(0) + let registered = () => undefined as { opencode?: unknown; slash?: unknown } | undefined + let command = () => undefined as KeymapCommand | undefined + + function Harness() { + const keymap = useKeymap() + const commands = Keymap.useCommands() + Keymap.createLayer(() => ({ + commands: [ + { + id: "demo.slash", + title: "Demo", + suggested: version() >= 0, + slash: { name: "demo", aliases: aliases(), arguments: true }, + run() {}, + }, + ], + })) + registered = () => + keymap.getCommandEntries({ visibility: "registered" }).find((entry) => entry.command.name === "demo.slash") + ?.command + command = () => commands().find((item) => item.id === "demo.slash") + return + } + + const app = await testRender(() => ( + + + + + + )) + try { + const first = registered() + setVersion(1) + expect(registered()?.opencode).toBe(first?.opencode) + expect(registered()?.slash).toBe(first?.slash) + + setAliases(["d", "dm"]) + expect(registered()?.opencode).not.toBe(first?.opencode) + expect(command()?.slash?.aliases).toEqual(["d", "dm"]) + } finally { + app.renderer.destroy() + } +}) + test("global commands stay reachable when the mode changes", async () => { const calls: string[] = [] let exercise = () => {}