Skip to content

Commit b3cf2d5

Browse files
authored
fix: clear the two warnings CI actually owns (#64)
* 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<McpOption>` 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. * 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.
1 parent 67d793d commit b3cf2d5

5 files changed

Lines changed: 47 additions & 18 deletions

File tree

‎Cargo.toml‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,4 +23,15 @@ hex = "0.4"
2323
thiserror = "2"
2424
uuid = { version = "1", features = ["v4"] }
2525
regex = "1"
26-
ts-rs = "12"
26+
# `no-serde-warnings` silences one diagnostic class: ts-rs warns and skips when
27+
# it meets a serde attribute it cannot model, and `McpParameter::options` uses
28+
# `deserialize_with` (see `mcp::model::de_options`) which ts-rs has no concept
29+
# of. Skipping it is correct -- the field always SERIALIZES back as a plain
30+
# list, so the emitted `Array<McpOption>` is right -- but the warning printed on
31+
# every `cargo clippy` run regardless.
32+
#
33+
# Nothing is lost by silencing it. The generated bindings are committed and CI's
34+
# "Fail on stale generated artifacts" step diffs them, so if ts-rs ever emits a
35+
# different shape for this field the build fails with an error instead of a
36+
# warning nobody reads.
37+
ts-rs = { version = "12", features = ["no-serde-warnings"] }

‎apps/desktop/src/renderer/shared/ui/DescriptionText/DescriptionText.tsx‎

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,10 @@
2020
* `DescriptionSpan[]` value satisfies it structurally.
2121
*/
2222
import { cx } from '../../lib';
23+
import { spansToKeyedParts } from './spansToKeyedParts';
24+
import type { DescriptionSpan } from './spansToKeyedParts';
2325
import './DescriptionText.scss';
2426

25-
/** One piece of a parsed description: plain text, or a link with its own
26-
* display text and target url. Structurally identical to the backend's
27-
* generated `DescriptionSpan`. */
28-
export type DescriptionSpan = { kind: 'text'; text: string } | { kind: 'link'; text: string; url: string };
29-
3027
export interface DescriptionTextProps {
3128
readonly spans: readonly DescriptionSpan[];
3229
/** Called with a link span's own `url` when its button is clicked. Never
@@ -40,15 +37,6 @@ export interface DescriptionTextProps {
4037
readonly 'data-testid'?: string;
4138
}
4239

43-
/** One span plus a stable React key. Keyed by position: spans never reorder
44-
* once parsed, so a position-based key stays distinct even when two link
45-
* spans repeat the same text and url. */
46-
export type KeyedDescriptionSpan = DescriptionSpan & { readonly key: string };
47-
48-
export function spansToKeyedParts(spans: readonly DescriptionSpan[]): KeyedDescriptionSpan[] {
49-
return spans.map((span, index) => ({ ...span, key: String(index) }));
50-
}
51-
5240
export function DescriptionText({ spans, onOpenLink, className, 'data-testid': testId }: DescriptionTextProps) {
5341
return (
5442
<span className={cx('sk-description', className)} data-testid={testId}>
Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,3 @@
11
export { DescriptionText } from './DescriptionText';
2-
export type { DescriptionTextProps, DescriptionSpan } from './DescriptionText';
2+
export type { DescriptionTextProps } from './DescriptionText';
3+
export type { DescriptionSpan } from './spansToKeyedParts';

apps/desktop/src/renderer/shared/ui/DescriptionText/DescriptionText.test.ts renamed to apps/desktop/src/renderer/shared/ui/DescriptionText/spansToKeyedParts.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { describe, expect, it } from 'vitest';
2-
import { spansToKeyedParts } from './DescriptionText';
3-
import type { DescriptionSpan } from './DescriptionText';
2+
import { spansToKeyedParts } from './spansToKeyedParts';
3+
import type { DescriptionSpan } from './spansToKeyedParts';
44

55
describe('spansToKeyedParts', () => {
66
it('keeps text and link spans in order with stable keys', () => {
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/**
2+
* The description span model and its React keying.
3+
*
4+
* Kept out of `DescriptionText.tsx` for two reasons that point the same way.
5+
* Renderer tests here are node-only -- no jsdom, no testing-library -- so a
6+
* component cannot be unit tested and pure logic has to live where a test can
7+
* reach it. And exporting a function from a file that also exports a component
8+
* breaks fast refresh, which is what `react-refresh/only-export-components`
9+
* reported for as long as this lived there.
10+
*
11+
* Same split, for the same reason, as
12+
* `features/skillInstall/lib/installSelection.ts`.
13+
*/
14+
15+
/** One piece of a parsed description: plain text, or a link with its own
16+
* display text and target url. Structurally identical to the backend's
17+
* generated `DescriptionSpan`, declared locally so this generic component has
18+
* no dependency on the `services` layer -- any concrete `DescriptionSpan[]`
19+
* value satisfies it. */
20+
export type DescriptionSpan = { kind: 'text'; text: string } | { kind: 'link'; text: string; url: string };
21+
22+
/** One span plus a stable React key. Keyed by position: spans never reorder
23+
* once parsed, so a position-based key stays distinct even when two link
24+
* spans repeat the same text and url. */
25+
export type KeyedDescriptionSpan = DescriptionSpan & { readonly key: string };
26+
27+
export function spansToKeyedParts(spans: readonly DescriptionSpan[]): KeyedDescriptionSpan[] {
28+
return spans.map((span, index) => ({ ...span, key: String(index) }));
29+
}

0 commit comments

Comments
 (0)