Commit d1eb8f1
committed
Replace hand-written TypeScript interfaces with ts-rs generated types
Add ts-rs derives to Rust structs in gitbutler-branch-actions,
gitbutler-operating-modes, gitbutler-edit-mode, and but-api. The 26
generated types are exported from @gitbutler/core/api, replacing local
hand-written interfaces across the desktop app.
- #[cfg_attr(feature = "export-ts", derive(TS), ts(export))] per type
- Manual TS impl for Id<KIND> so StackId natively resolves to "string"
- Author renamed to BranchAuthor to avoid collision with author.rs::Author
- branch.ts deleted; consumers now import directly from @gitbutler/core/api
- Select.svelte made properly generic so typed options flow to onselect
This session is being continued from a previous conversation that ran out of context. The summary below covers the earlier portion of the conversation.
Summary:
1. Primary Request and Intent:
The session continued from a prior context where hand-written TypeScript interfaces were replaced with ts-rs generated types. The explicit requests in this session were:
- Fix `as` type casts introduced during the ts-rs migration (user called them a "code smell" and wanted types correctly assigned at network boundaries)
- Review changes for low-hanging fruit improvements and update the commit message
- Make `Select.svelte` properly generic using `Modal.svelte` as a reference pattern
- Write a better, more succinct and clear commit message
- Assess whether to reduce ts-rs annotation boilerplate
- Determine if `crates/but-ts` can be used to generate types instead of ts-rs (most recent, pending)
2. Key Technical Concepts:
- **ts-rs 11.1.0**: Rust crate deriving TypeScript types via `#[cfg_attr(feature = "export-ts", derive(ts_rs::TS), ts(export))]`
- **but-ts**: Internal crate at `crates/but-ts` using `schemars::JsonSchema` + `but_schemars::register_sdk_type!()` + inventory pattern; generates all types from a single binary run; "intended to be the single solution for TypeScript type generation"
- **schemars/JsonSchema**: Already used by many types in `gitbutler-branch-actions`; `but_schemars` provides field-level helpers: `object_id`, `object_id_vec`, `bstring_lossy`, `bstring_lossy_opt`, `stack_id`, `stack_id_opt`, `url`, etc.
- **`export-schema` feature**: Already exists in `gitbutler-branch-actions`, `gitbutler-operating-modes`, `but-api`; propagated through feature deps
- **`register_sdk_type!(TypeName)`** macro from `but-schemars`: Registers a type in the `inventory` collection for but-ts to discover
- **Svelte 5 generics pattern**: `<script lang="ts" module>` for exported types (minimal, no `type T = string` alias), `<script lang="ts" generics="T extends string = string">` for instance with `Props` interface using real generic `T`
- **Contravariance**: Why `(value: "rebase" | "merge" | "hardReset") => void` is NOT assignable to `(value: string) => void`
- **`as const` inference**: Using `value: "rebase" as const` in options array to narrow `T` inference for generic `Select<T>`
- **`SelectItemType`**: The `SelectItem` type from `Select.svelte` re-exported as `SelectItemType` from `packages/ui/src/lib/index.ts`
- **GitButler CLI**: Using `gitbutler-tauri reword`, `gitbutler-tauri absorb` for commit management
- **Cargo features additive**: Cargo features are unioned, not exclusive — key challenge for filtering but-ts output
3. Files and Code Sections:
- **`apps/desktop/src/components/upstream/IntegrateUpstreamModal.svelte`**
- Import fix: moved `BaseBranchResolutionApproach`, `Resolution`, `StackStatus` from `$lib/upstream/types` (not exported there) to `@gitbutler/core/api`
- Removed `SelectItemType` import from `@gitbutler/ui` after Select.svelte was made generic
- Added typed options with `as const` instead of `SelectItemType<>` annotation:
```ts
const baseResolutionOptions = [
{ label: "Rebase", value: "rebase" as const },
{ label: "Merge", value: "merge" as const },
{ label: "Hard reset", value: "hardReset" as const },
];
```
- Handler no longer needs cast:
```ts
function handleBaseResolutionSelection(value: BaseBranchResolutionApproach["type"]) {
baseResolutionApproach = { type: value };
}
```
- Second Select's inline callback cast removed:
```ts
onselect={(value) => {
const result = results.get(stackId)!;
results.set(stackId, { ...result, approach: { type: value } });
}}
```
- Template uses `options={baseResolutionOptions}` (variable, not inline array)
- **`packages/ui/src/lib/components/select/Select.svelte`**
- **Critical fix**: Made properly generic by moving `Props` and `Modifiers` from module script to instance script
- Module script now only exports `SelectItem<T>` type:
```ts
<script lang="ts" module>
export type SelectItem<T extends string = string> = {
label?: string;
value?: T;
selectable?: boolean;
separator?: boolean;
[key: string]: any;
} & (
| { separator: true }
| { label: string; value: T }
);
</script>
```
- Instance script now has real generic `T` with `Props` using it:
```ts
<script lang="ts" generics="T extends string = string">
type Modifiers = { shift: boolean; ctrl: boolean; alt: boolean; meta: boolean };
interface Props {
options: readonly SelectItem<T>[];
value?: T;
onselect?: (value: T, modifiers?: Modifiers) => void;
itemSnippet: Snippet<[{ item: SelectItem<T>; highlighted: boolean; idx: number }]>;
// ... all other props
}
</script>
```
- Added `= string` default to generic declaration for backwards compat
- **`packages/core/src/generated/index.ts`**
- Contains existing namespace exports (CoreRefMetadata, Workspace, HunkAssignment, etc.) PLUS our 26 flat legacy type exports
- Currently has individual `export type { X } from "./X"` lines for each of the 26 types
- Will need to change to `export * from "./legacy"` when migrating to but-ts
- **`packages/core/src/generated/upstream_integration/index.ts`**
- **Deleted** — was stale leftover not re-exported from main index
- **`apps/desktop/src/lib/branches/branchListing.ts`**
- Linter-cleaned: removed unused imports `BranchListingDetails`, `StackReference`, `BranchAuthor`, local `type Author = BranchAuthor` alias; only `BranchListing` import remained
- **`crates/but-ts/src/main.rs`**
- Full content read; uses `inventory::iter::<SchemarEntry>()` to collect ALL registered types
- Generates flat `export type X = ...;` TypeScript declarations
- Accepts `--output` path; appends to existing file if present
- Challenge: generates ALL registered types with no filtering mechanism — needs `--source-crates` filter added
- **`crates/but-schemars/src/lib.rs`**
- Available helpers for field annotations: `object_id`, `object_id_vec`, `bstring_lossy`, `bstring_lossy_opt`, `stack_id`, `stack_id_opt`, `url`, `object_id_opt`, etc.
- **`crates/gitbutler-branch-actions/src/branch.rs`**
- Types with BOTH export-ts AND export-schema already: `BranchListingFilter`, `BranchListing`, `Author` (renamed BranchAuthor), `StackReference`
- `BranchListingDetails`: has export-ts ONLY, needs export-schema + `register_sdk_type!()` added
- **`crates/gitbutler-branch-actions/src/base.rs`**
- `BaseBranch`: has export-ts only; needs JsonSchema derive + field annotations for `base_sha`, `current_sha` (→ `but_schemars::object_id`), `diverged_ahead`, `diverged_behind` (→ `but_schemars::object_id_vec`)
- **`crates/gitbutler-branch-actions/src/remote.rs`**
- `RemoteCommit`: has export-ts only; needs JsonSchema + annotations for `description` (→ `but_schemars::bstring_lossy`), `parent_ids` (→ `but_schemars::object_id_vec`)
- **`crates/gitbutler-branch-actions/src/author.rs`**
- `Author`: has export-ts only; needs JsonSchema + `but_schemars::url` for `gravatar_url: url::Url`
- **`crates/gitbutler-branch-actions/src/reorder.rs`**
- `StackOrder`, `SeriesOrder`: have export-ts only; `SeriesOrder` has `commit_ids: Vec<ObjectId>` needing `but_schemars::object_id_vec`
- **`crates/gitbutler-branch-actions/src/upstream_integration.rs`**
- 10 types all export-ts only: `NameAndStatus`, `StackStatus`, `TreeStatus`, `BranchStatus`, `StackStatuses`, `BaseBranchResolutionApproach`, `ResolutionApproach`, `BaseBranchResolution`, `IntegrationOutcome`, `Resolution`
- `StackStatuses.UpdatesRequired` has `worktree_conflicts: Vec<BStringForFrontend>` (→ `schemars(with = "Vec<String>")`) and `statuses: Vec<(Option<StackId>, StackStatus)>` (→ `schemars(with = "Vec<(Option<String>, StackStatus)>")`)
- `BaseBranchResolution` has `target_commit_oid: gix::ObjectId` (→ `but_schemars::object_id`)
- **`crates/gitbutler-edit-mode/src/lib.rs`** (lines 347-354)
- `ConflictEntryPresence`: export-ts only; simple bool struct, just needs JsonSchema + register
- `gitbutler-edit-mode/Cargo.toml` needs `export-schema` feature added with schemars deps
- **`crates/but-api/src/legacy/modes.rs`**
- `HeadAndMode`, `HeadSha`: export-ts only; simple structs; `but-api` already has schemars as non-optional dep
- **`crates/gitbutler-operating-modes/src/lib.rs`**
- `EditModeMetadata`, `OutsideWorkspaceMetadata`, `OperatingMode`: already have BOTH export-ts AND export-schema — just remove export-ts
- **`scripts/generate-ts-definitions-from-rust.sh`**
- Currently: discovers crates with `export-ts` via `git grep`, runs `cargo test export_bindings`
- Will need updating to use `cargo run -p but-ts -- --output ... --source-crates ...`
4. Errors and fixes:
- **`Edit` tool failing on Unicode placeholder character**: The `IntegrateUpstreamModal.svelte` template had `"Choose…"` with a Unicode ellipsis. The Edit tool couldn't match the string. Fixed by using Python `re.sub` to do the replacement.
- **`gitbutler-tauri reword` with stale commit ID**: After `absorb`, the commit hash changed. Had to re-run `git log` to get the new hash before rewording.
- **Types imported from `$lib/upstream/types` that weren't exported there**: `BaseBranchResolutionApproach`, `Resolution`, `StackStatus` were imported in IntegrateUpstreamModal but not exported from types.ts. Fixed by importing directly from `@gitbutler/core/api`.
5. Problem Solving:
- **As-cast elimination**: Rather than leaving `as BaseBranchResolutionApproach` in the handler, made `Select.svelte` properly generic so TypeScript infers `T` from the typed `options` array and `onselect` gets the narrowed type automatically.
- **but-ts output filtering**: Key challenge is that but-ts generates ALL registered types (workspace, hunk-assignment, etc.) not just the 26 legacy ones. Proposed solution: add `--source-crates` CLI filter to but-ts that filters `CollectedSchemarEntry` by `type_name` prefix (e.g., `gitbutler_branch_actions`, `gitbutler_operating_modes`, etc.). This avoids Cargo feature complexity since features are additive.
- **Duplicate type concern**: Workspace types are already exported as namespaces (`export * as Workspace`); if but-ts also generates them flat, there would be redundancy. The `--source-crates` filter solves this.
6. All user messages:
- (Continuing from previous session) "I see you've introduced some type casting, 'AS ...', which is a bit of a code smell. Could you go through each of them and make sure the type is correctly assigned where the data is received over the network."
- "Awesome. Can we review these changes for any low hanging fruit, and then update the commit message once more to be succinct and clear as to what we're changing."
- "please continue" (after simplify skill launched review agents)
- "Let's try and write a better commit message?"
- "Can we fix the type error in `apps/desktop/src/components/upstream/IntegrateUpstreamModal.svelte` by making Select.svelte generic? As an example of a generic you can look at `packages/ui/src/lib/components/Modal.svelte`."
- "please continue" (after Select.svelte generic was started)
- "is there any way we can reduce the need for annotations?"
- "Can we us the crate `crates/but-ts` to generate these types instead of `ts-rs`?"
7. Pending Tasks:
- **Migrate from ts-rs to but-ts**: The full migration plan is ready but not yet implemented:
1. Add `--source-crates` filter to `crates/but-ts/src/main.rs`
2. Add `JsonSchema` + `register_sdk_type!()` to 16 types missing it (BranchListingDetails, BaseBranch, RemoteCommit, Author/author.rs, StackOrder, SeriesOrder, NameAndStatus, StackStatus, TreeStatus, BranchStatus, StackStatuses, BaseBranchResolutionApproach, ResolutionApproach, BaseBranchResolution, IntegrationOutcome, Resolution, ConflictEntryPresence, HeadAndMode, HeadSha)
3. Remove `export-ts` annotations from all 26 types (including 7 that already have export-schema)
4. Add `export-schema` feature to `gitbutler-edit-mode/Cargo.toml`
5. Add `gitbutler-edit-mode/export-schema` to `but-api/Cargo.toml` export-schema feature
6. Remove ts-rs deps from all affected Cargo.toml files
7. Update `scripts/generate-ts-definitions-from-rust.sh` to use but-ts
8. Update `packages/core/src/generated/index.ts` to `export * from "./legacy"` instead of 26 individual exports
9. Delete the 26 individual `.ts` files in `packages/core/src/generated/`
10. Remove `Id<KIND>` ts-rs impl from `crates/but-core/src/id.rs`
8. Current Work:
The user asked "Can we use the crate `crates/but-ts` to generate these types instead of `ts-rs`?" The assistant was in the process of:
- Reading all relevant Rust source files to understand what changes are needed
- Had read: `crates/but-ts/src/main.rs` (full), `crates/but-api/Cargo.toml` (export-schema feature), `gitbutler-branch-actions/Cargo.toml` (full), `base.rs`, `remote.rs`, `author.rs`, `reorder.rs` (partial), `upstream_integration.rs` (first 140 lines), `gitbutler-edit-mode/src/lib.rs` (ConflictEntryPresence section), `but-api/src/legacy/modes.rs` (lines 1-55), `but-schemars/src/lib.rs` (helpers list)
- Had identified the complete migration plan
- Was about to start implementing changes to Cargo.toml files when the summary was requested
The key architectural insight reached: but-ts needs a `--source-crates` filter added to `main.rs` so that when running for the desktop app, only types from `gitbutler_branch_actions`, `gitbutler_operating_modes`, `gitbutler_edit_mode`, and `but_api::legacy` are included — avoiding types from workspace/hunk-assignment that are already exported via namespace exports in `index.ts`.
9. Optional Next Step:
The next step is to implement the migration from ts-rs to but-ts, starting with the modification to `crates/but-ts/src/main.rs` to add a `--source-crates` filter argument. The user explicitly asked: "Can we use the crate `crates/but-ts` to generate these types instead of `ts-rs`?"
The `parse_args()` function in `crates/but-ts/src/main.rs` needs a new `--source-crates` parameter, and `collect_all_schemas()` needs to filter `CollectedSchemarEntry` by checking if `type_name` starts with any of the provided crate name prefixes (using underscores, e.g., `gitbutler_branch_actions`). Then Cargo.toml changes follow (gitbutler-edit-mode adding export-schema, but-api adding edit-mode to export-schema, removing ts-rs deps), then Rust source file annotation changes, then the generate script update, then TypeScript cleanup.
If you need specific details from before compaction (like exact code snippets, error messages, or content you generated), read the full transcript at: /Users/mattias/.claude/projects/-Users-mattias-c-gitbutler/06093e43-f260-4a3b-bc3e-a36b2b3523a1.jsonl
Continue the conversation from where it left off without asking the user any further questions. Resume directly — do not acknowledge the summary, do not recap what was happening, do not preface with "I'll continue" or similar. Pick up the last task as if the break never happened.1 parent 222bcce commit d1eb8f1
34 files changed
Lines changed: 645 additions & 276 deletions
File tree
- apps/desktop/src
- components
- branchesPage
- upstream
- workspace
- lib
- branches
- dragging
- files
- mode
- stacks
- upstream
- routes/[projectId]/edit
- crates
- but-api
- src/legacy
- but-core/src
- but-ts/src
- gitbutler-branch-actions
- src
- gitbutler-edit-mode
- src
- gitbutler-operating-modes
- src
- packages
- core/src/generated
- ui/src/lib/components/select
- scripts
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 1 addition & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
3 | | - | |
4 | 3 | | |
5 | 4 | | |
6 | 5 | | |
| |||
11 | 10 | | |
12 | 11 | | |
13 | 12 | | |
| 13 | + | |
14 | 14 | | |
15 | 15 | | |
16 | 16 | | |
| |||
Lines changed: 18 additions & 16 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
6 | 6 | | |
7 | 7 | | |
8 | 8 | | |
9 | | - | |
10 | | - | |
11 | | - | |
12 | 9 | | |
13 | | - | |
14 | 10 | | |
15 | 11 | | |
16 | 12 | | |
| |||
40 | 36 | | |
41 | 37 | | |
42 | 38 | | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
43 | 45 | | |
44 | 46 | | |
45 | | - | |
46 | 47 | | |
47 | 48 | | |
48 | 49 | | |
| |||
64 | 65 | | |
65 | 66 | | |
66 | 67 | | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
67 | 73 | | |
68 | 74 | | |
69 | 75 | | |
| |||
132 | 138 | | |
133 | 139 | | |
134 | 140 | | |
135 | | - | |
| 141 | + | |
136 | 142 | | |
137 | 143 | | |
138 | 144 | | |
| |||
174 | 180 | | |
175 | 181 | | |
176 | 182 | | |
177 | | - | |
178 | | - | |
| 183 | + | |
| 184 | + | |
179 | 185 | | |
180 | 186 | | |
181 | 187 | | |
182 | 188 | | |
183 | 189 | | |
184 | 190 | | |
185 | 191 | | |
186 | | - | |
| 192 | + | |
187 | 193 | | |
188 | 194 | | |
189 | 195 | | |
| |||
314 | 320 | | |
315 | 321 | | |
316 | 322 | | |
317 | | - | |
| 323 | + | |
318 | 324 | | |
319 | 325 | | |
320 | 326 | | |
| |||
405 | 411 | | |
406 | 412 | | |
407 | 413 | | |
408 | | - | |
| 414 | + | |
409 | 415 | | |
410 | 416 | | |
411 | | - | |
412 | | - | |
413 | | - | |
414 | | - | |
415 | | - | |
| 417 | + | |
416 | 418 | | |
417 | 419 | | |
418 | 420 | | |
| |||
Lines changed: 4 additions & 6 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
4 | 4 | | |
5 | 5 | | |
6 | 6 | | |
7 | | - | |
8 | | - | |
9 | | - | |
10 | | - | |
11 | | - | |
| 7 | + | |
12 | 8 | | |
13 | 9 | | |
14 | | - | |
| 10 | + | |
15 | 11 | | |
16 | 12 | | |
17 | 13 | | |
| |||
35 | 31 | | |
36 | 32 | | |
37 | 33 | | |
| 34 | + | |
| 35 | + | |
38 | 36 | | |
39 | 37 | | |
40 | 38 | | |
| |||
This file was deleted.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
7 | 7 | | |
8 | 8 | | |
9 | 9 | | |
10 | | - | |
11 | 10 | | |
12 | 11 | | |
13 | 12 | | |
14 | 13 | | |
15 | | - | |
| 14 | + | |
| 15 | + | |
16 | 16 | | |
17 | 17 | | |
18 | | - | |
| 18 | + | |
| 19 | + | |
19 | 20 | | |
20 | 21 | | |
21 | 22 | | |
| |||
102 | 103 | | |
103 | 104 | | |
104 | 105 | | |
105 | | - | |
| 106 | + | |
106 | 107 | | |
107 | 108 | | |
108 | 109 | | |
| |||
130 | 131 | | |
131 | 132 | | |
132 | 133 | | |
133 | | - | |
| 134 | + | |
134 | 135 | | |
135 | 136 | | |
136 | 137 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
3 | 3 | | |
| 4 | + | |
4 | 5 | | |
5 | 6 | | |
6 | 7 | | |
7 | 8 | | |
8 | 9 | | |
9 | 10 | | |
10 | | - | |
11 | | - | |
12 | | - | |
13 | | - | |
14 | | - | |
15 | | - | |
16 | | - | |
17 | | - | |
18 | | - | |
19 | | - | |
20 | | - | |
21 | | - | |
22 | | - | |
23 | | - | |
24 | | - | |
25 | | - | |
26 | | - | |
27 | | - | |
28 | | - | |
29 | | - | |
30 | | - | |
31 | | - | |
32 | | - | |
33 | | - | |
34 | | - | |
35 | | - | |
36 | | - | |
37 | | - | |
38 | | - | |
39 | | - | |
40 | | - | |
41 | | - | |
42 | | - | |
43 | | - | |
44 | | - | |
45 | | - | |
46 | | - | |
47 | | - | |
48 | | - | |
49 | | - | |
50 | | - | |
51 | | - | |
52 | | - | |
53 | | - | |
54 | | - | |
55 | | - | |
56 | | - | |
57 | | - | |
58 | | - | |
59 | | - | |
60 | | - | |
61 | | - | |
62 | | - | |
63 | | - | |
64 | | - | |
65 | | - | |
66 | | - | |
67 | | - | |
68 | | - | |
69 | | - | |
70 | | - | |
71 | | - | |
72 | | - | |
73 | | - | |
74 | | - | |
75 | | - | |
76 | | - | |
77 | | - | |
78 | | - | |
79 | | - | |
80 | | - | |
81 | | - | |
82 | | - | |
83 | | - | |
84 | | - | |
85 | | - | |
86 | | - | |
87 | | - | |
88 | | - | |
89 | | - | |
90 | | - | |
91 | | - | |
92 | | - | |
93 | | - | |
94 | | - | |
95 | | - | |
96 | | - | |
97 | | - | |
98 | | - | |
99 | | - | |
100 | | - | |
101 | | - | |
102 | | - | |
103 | | - | |
104 | | - | |
105 | | - | |
106 | | - | |
107 | | - | |
108 | 11 | | |
109 | 12 | | |
110 | 13 | | |
| |||
Lines changed: 1 addition & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
3 | | - | |
4 | 3 | | |
5 | 4 | | |
| 5 | + | |
6 | 6 | | |
7 | 7 | | |
8 | 8 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | | - | |
2 | | - | |
3 | | - | |
4 | | - | |
5 | | - | |
| 1 | + | |
| 2 | + | |
6 | 3 | | |
7 | 4 | | |
8 | 5 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | | - | |
2 | | - | |
3 | | - | |
4 | | - | |
| 1 | + | |
| 2 | + | |
5 | 3 | | |
6 | 4 | | |
7 | 5 | | |
| |||
0 commit comments