From efaebb9b5310ccfacd7877ab4b56c10cd57d09f5 Mon Sep 17 00:00:00 2001 From: Lorem Dev Date: Wed, 9 Sep 2026 09:25:36 +0200 Subject: [PATCH 1/2] build: silence the ts-rs serde-attribute warning Every `cargo clippy` run printed "failed to parse serde attribute". `McpParameter::options` uses `deserialize_with` (see `mcp::model::de_options`) to accept a YAML mapping as well as a list, and ts-rs has no concept of that attribute, so it warns and skips it. Skipping is the right thing: the field always SERIALIZES back as a plain list, so the emitted `Array` is correct. The warning reported a decision that was already right, on every run. Nothing is lost by silencing it. The generated bindings are committed and CI diffs them ("Fail on stale generated artifacts"), so if ts-rs ever emits a different shape for this field the build fails with an error rather than a warning nobody reads. Verified the bindings are byte-identical with the feature on. --- Cargo.toml | 13 ++++++++++++- ...iptionText.test.ts => spansToKeyedParts.test.ts} | 0 2 files changed, 12 insertions(+), 1 deletion(-) rename apps/desktop/src/renderer/shared/ui/DescriptionText/{DescriptionText.test.ts => spansToKeyedParts.test.ts} (100%) diff --git a/Cargo.toml b/Cargo.toml index a73910f4..9be11692 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -23,4 +23,15 @@ hex = "0.4" thiserror = "2" uuid = { version = "1", features = ["v4"] } regex = "1" -ts-rs = "12" +# `no-serde-warnings` silences one diagnostic class: ts-rs warns and skips when +# it meets a serde attribute it cannot model, and `McpParameter::options` uses +# `deserialize_with` (see `mcp::model::de_options`) which ts-rs has no concept +# of. Skipping it is correct -- the field always SERIALIZES back as a plain +# list, so the emitted `Array` is right -- but the warning printed on +# every `cargo clippy` run regardless. +# +# Nothing is lost by silencing it. The generated bindings are committed and CI's +# "Fail on stale generated artifacts" step diffs them, so if ts-rs ever emits a +# different shape for this field the build fails with an error instead of a +# warning nobody reads. +ts-rs = { version = "12", features = ["no-serde-warnings"] } diff --git a/apps/desktop/src/renderer/shared/ui/DescriptionText/DescriptionText.test.ts b/apps/desktop/src/renderer/shared/ui/DescriptionText/spansToKeyedParts.test.ts similarity index 100% rename from apps/desktop/src/renderer/shared/ui/DescriptionText/DescriptionText.test.ts rename to apps/desktop/src/renderer/shared/ui/DescriptionText/spansToKeyedParts.test.ts From 24dd142876c009219b8ec2a9aa4175a5c1e6c1ed Mon Sep 17 00:00:00 2001 From: Lorem Dev Date: Wed, 9 Sep 2026 09:25:37 +0200 Subject: [PATCH 2/2] refactor: move span keying out of the DescriptionText component `pnpm lint` reported one warning for as long as this file has existed: `react-refresh/only-export-components`, because `spansToKeyedParts` was exported from a file that also exports a component. The export was not gratuitous. Renderer tests here are node-only -- no jsdom, no testing-library -- so a component cannot be unit tested, and the keying rule (position-based, so two identical link spans stay distinct) has to live where a test can reach it. The fix is to put it in its own module rather than to stop testing it, which is the same split `features/skillInstall/lib/installSelection.ts` already uses. `DescriptionSpan` moves with it: the type is data the function transforms, not part of the component's API, and the folder barrel now sources it from its owner. Other layers were already importing their own `DescriptionSpan` from `@/services/bridge`, so nothing outside this folder changes. `pnpm lint` now reports nothing at all. --- .../ui/DescriptionText/DescriptionText.tsx | 16 ++-------- .../shared/ui/DescriptionText/index.ts | 3 +- .../DescriptionText/spansToKeyedParts.test.ts | 4 +-- .../ui/DescriptionText/spansToKeyedParts.ts | 29 +++++++++++++++++++ 4 files changed, 35 insertions(+), 17 deletions(-) create mode 100644 apps/desktop/src/renderer/shared/ui/DescriptionText/spansToKeyedParts.ts diff --git a/apps/desktop/src/renderer/shared/ui/DescriptionText/DescriptionText.tsx b/apps/desktop/src/renderer/shared/ui/DescriptionText/DescriptionText.tsx index 7944f100..13591178 100644 --- a/apps/desktop/src/renderer/shared/ui/DescriptionText/DescriptionText.tsx +++ b/apps/desktop/src/renderer/shared/ui/DescriptionText/DescriptionText.tsx @@ -20,13 +20,10 @@ * `DescriptionSpan[]` value satisfies it structurally. */ import { cx } from '../../lib'; +import { spansToKeyedParts } from './spansToKeyedParts'; +import type { DescriptionSpan } from './spansToKeyedParts'; import './DescriptionText.scss'; -/** One piece of a parsed description: plain text, or a link with its own - * display text and target url. Structurally identical to the backend's - * generated `DescriptionSpan`. */ -export type DescriptionSpan = { kind: 'text'; text: string } | { kind: 'link'; text: string; url: string }; - export interface DescriptionTextProps { readonly spans: readonly DescriptionSpan[]; /** Called with a link span's own `url` when its button is clicked. Never @@ -40,15 +37,6 @@ export interface DescriptionTextProps { readonly 'data-testid'?: string; } -/** One span plus a stable React key. Keyed by position: spans never reorder - * once parsed, so a position-based key stays distinct even when two link - * spans repeat the same text and url. */ -export type KeyedDescriptionSpan = DescriptionSpan & { readonly key: string }; - -export function spansToKeyedParts(spans: readonly DescriptionSpan[]): KeyedDescriptionSpan[] { - return spans.map((span, index) => ({ ...span, key: String(index) })); -} - export function DescriptionText({ spans, onOpenLink, className, 'data-testid': testId }: DescriptionTextProps) { return ( diff --git a/apps/desktop/src/renderer/shared/ui/DescriptionText/index.ts b/apps/desktop/src/renderer/shared/ui/DescriptionText/index.ts index 68bbeb1b..1e7d09a8 100644 --- a/apps/desktop/src/renderer/shared/ui/DescriptionText/index.ts +++ b/apps/desktop/src/renderer/shared/ui/DescriptionText/index.ts @@ -1,2 +1,3 @@ export { DescriptionText } from './DescriptionText'; -export type { DescriptionTextProps, DescriptionSpan } from './DescriptionText'; +export type { DescriptionTextProps } from './DescriptionText'; +export type { DescriptionSpan } from './spansToKeyedParts'; diff --git a/apps/desktop/src/renderer/shared/ui/DescriptionText/spansToKeyedParts.test.ts b/apps/desktop/src/renderer/shared/ui/DescriptionText/spansToKeyedParts.test.ts index 6c1790a7..1d28c6af 100644 --- a/apps/desktop/src/renderer/shared/ui/DescriptionText/spansToKeyedParts.test.ts +++ b/apps/desktop/src/renderer/shared/ui/DescriptionText/spansToKeyedParts.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from 'vitest'; -import { spansToKeyedParts } from './DescriptionText'; -import type { DescriptionSpan } from './DescriptionText'; +import { spansToKeyedParts } from './spansToKeyedParts'; +import type { DescriptionSpan } from './spansToKeyedParts'; describe('spansToKeyedParts', () => { it('keeps text and link spans in order with stable keys', () => { diff --git a/apps/desktop/src/renderer/shared/ui/DescriptionText/spansToKeyedParts.ts b/apps/desktop/src/renderer/shared/ui/DescriptionText/spansToKeyedParts.ts new file mode 100644 index 00000000..7627c063 --- /dev/null +++ b/apps/desktop/src/renderer/shared/ui/DescriptionText/spansToKeyedParts.ts @@ -0,0 +1,29 @@ +/** + * The description span model and its React keying. + * + * Kept out of `DescriptionText.tsx` for two reasons that point the same way. + * Renderer tests here are node-only -- no jsdom, no testing-library -- so a + * component cannot be unit tested and pure logic has to live where a test can + * reach it. And exporting a function from a file that also exports a component + * breaks fast refresh, which is what `react-refresh/only-export-components` + * reported for as long as this lived there. + * + * Same split, for the same reason, as + * `features/skillInstall/lib/installSelection.ts`. + */ + +/** One piece of a parsed description: plain text, or a link with its own + * display text and target url. Structurally identical to the backend's + * generated `DescriptionSpan`, declared locally so this generic component has + * no dependency on the `services` layer -- any concrete `DescriptionSpan[]` + * value satisfies it. */ +export type DescriptionSpan = { kind: 'text'; text: string } | { kind: 'link'; text: string; url: string }; + +/** One span plus a stable React key. Keyed by position: spans never reorder + * once parsed, so a position-based key stays distinct even when two link + * spans repeat the same text and url. */ +export type KeyedDescriptionSpan = DescriptionSpan & { readonly key: string }; + +export function spansToKeyedParts(spans: readonly DescriptionSpan[]): KeyedDescriptionSpan[] { + return spans.map((span, index) => ({ ...span, key: String(index) })); +}