Skip to content

Commit 78b1e94

Browse files
authored
Merge pull request #396 from orbyn-dev/refactor/web-feature-api
refactor(web): give every feature one api front door
2 parents 2513611 + f189e1f commit 78b1e94

131 files changed

Lines changed: 229 additions & 153 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎docs/refactor.md‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ Notes:
106106
| W1 | Dead code and shared helpers | done |
107107
| W2 | CSS ownership: no class defined twice | open, needs the owner's go-ahead |
108108
| W3 | Break feature cycles; `features/content` and `index.ts` front doors | done |
109-
| W4 | Per-feature `api.ts` and hooks | open (not started) |
109+
| W4 | Per-feature `api.ts` and hooks | in progress: front doors done |
110110
| W5 | Giant components into sub-folders (composer, trail, sidebar, panels, settings, pulls, usage) | done |
111111
| W6 | Naming pass and `web/README.md` | `web/README.md` exists (W7a); the naming pass is open |
112112
| W7a | Product and generic tiers for shared web code (`product/`) | done |
@@ -220,6 +220,13 @@ The backend (B) and web (W) tracks can run in parallel.
220220
transport and its tests use and `source_control::git` only `is_git_repo`; the rest is
221221
`pub(crate)`, with no call site changed.
222222

223+
- W4 (per-feature API doors): started. Every feature now has `api.ts`: it re-exports the
224+
client pieces that feature uses, and 119 files import it instead of `api/`. Value imports
225+
from `api/client`, `shell`, `pulls`, `sse`, `token` and `origin` in feature code went from
226+
158 to 0; type-only imports of `api/types` stay, since they carry no call. Named
227+
operations and hooks are next: the maintenance page, `BrowserPanel`, the terminal drawer
228+
and the settings pages still call `request(...)` with endpoint paths through that door.
229+
223230
- W7a (product and generic tiers): done for the shared folders; front-door helpers are open (see
224231
the audit below). `state/` became `product/state/`, and `lib/timeline` became `product/timeline`.
225232
The shared files that carry product vocabulary and have two or more feature users moved to
@@ -301,7 +308,7 @@ have no baseline.
301308
| Unit | Work | Size and notes |
302309
| --- | --- | --- |
303310
| W2 | CSS ownership: `styles/thread.css` (about 4,500 lines, mostly panels and onboarding rules), `pages.css`, `shell.css`, `base.css` move next to the components they style; shared pieces (`btn`, `dialog`, `pill`, `empty` and the like, defined in up to 10 files) are defined once in `ui/` | **Needs the owner's go-ahead.** The tests cannot see a changed cascade. Compare screenshots before and after in Light, Dark and Dim for every moved rule, and keep the CSS load order identical (compare the first-import order from `main.tsx`, as the W7a review did). |
304-
| W4 | Per-feature `api.ts` and hooks: components stop calling the API client directly (about 150 direct imports, for example Maintenance, BrowserPanel, the terminal drawer, the settings pages) | Web. Not started. |
311+
| W4 | Per-feature `api.ts` and hooks: named operations replace the raw `request(...)` calls the feature front doors still re-export | Web. Front doors are in (119 files). Next: Maintenance, `BrowserPanel`, the terminal drawer and the settings pages. |
305312
| W6 | Naming pass: Page is a route, Panel is a side-panel tab, Dialog is a modal, no "Pane" (`SurfacePane`, `SideChatPane`, `PullsPage` against `PullPage`); keep `web/README.md` in step | Do after W2 so files are renamed once. |
306313
| Front-door TODO rows | Hoist the helper exports in the audit table (one other feature uses each) to `product/` or `lib/` when a second feature needs them; hoist `copyText` to `lib/` | Opportunistic. |
307314
| Closeout | Turn any remaining baselined rule into a hard error, run a packaged-app build and walk-through (`cd desktop && npm run build -- --bundles app`), run the worktree cleanup report | The packaged-app check has not been run since the logo, tools, picker and tab strip merged into main. |

‎web/e2e/real/settings-scopes.spec.ts‎

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { expect, test } from "@playwright/test";
1+
import { expect, test, type Route } from "@playwright/test";
22
import { execFileSync } from "node:child_process";
33
import { writeFileSync } from "node:fs";
44
import { join } from "node:path";
@@ -55,16 +55,28 @@ test("logical project settings resolve Mixed and reset explicit overrides throug
5555
await prefix.fill("one-only"); await prefix.press("Tab");
5656
await expect.poll(async () => (await pref(one.id, "branch_name_prefix"))?.value).toBe("one-only");
5757
expect((await pref(two.id, "branch_name_prefix"))?.value).toBe("shared");
58-
const branchPreferences = page.waitForResponse(async (response) => {
59-
if (response.request().method() !== "POST" || new URL(response.url()).pathname !== "/api/settings/preferences/query" || !response.ok()) return false;
60-
const query = response.request().postDataJSON() as ScopedPreferencesQuery;
61-
if (query.scope.selection !== undefined || !query.keys?.includes("branch_name_prefix")) return false;
62-
const result = await response.json() as ScopedPreferencesResult;
63-
const branch = result.rows.find((entry) => entry.key === "branch_name_prefix");
64-
return result.catalog.selection.kind === "all_projects"
65-
&& [one.id, two.id].every((id) => branch?.overrides.some((entry) => entry.project_id === id));
66-
}, { timeout: 10_000 });
67-
await Promise.all([branchPreferences, page.goto(`${service.page}#/settings/worktrees`)]);
58+
// Read the body in the route, not in a waitForResponse predicate: the navigation that
59+
// triggers the query can evict the response body before the predicate can parse it.
60+
const preferenceQuery = "**/api/settings/preferences/query";
61+
let branchResult: ScopedPreferencesResult | undefined;
62+
const captureBranchPreferences = async (route: Route) => {
63+
const request = route.request();
64+
const query = request.postDataJSON() as ScopedPreferencesQuery;
65+
if (request.method() !== "POST" || query.scope.selection !== undefined || !query.keys?.includes("branch_name_prefix")) {
66+
await route.continue();
67+
return;
68+
}
69+
const response = await route.fetch();
70+
const body = await response.text();
71+
if (response.ok()) branchResult = JSON.parse(body) as ScopedPreferencesResult;
72+
await route.fulfill({ response, body });
73+
};
74+
await page.route(preferenceQuery, captureBranchPreferences);
75+
await page.goto(`${service.page}#/settings/worktrees`);
76+
await expect.poll(() => branchResult?.catalog.selection.kind, { timeout: 10_000 }).toBe("all_projects");
77+
await page.unroute(preferenceQuery, captureBranchPreferences);
78+
const branch = branchResult!.rows.find((entry) => entry.key === "branch_name_prefix");
79+
expect([one.id, two.id].every((id) => branch?.overrides.some((entry) => entry.project_id === id))).toBe(true);
6880
const row = page.locator("#worktrees-branch-prefix");
6981
await row.getByRole("button", { name: "Default", exact: true }).click();
7082
await expect(row.getByText("Overridden by", { exact: true })).toBeVisible();

‎web/src/features/content/api.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
// The one door this feature uses to reach the API client. Named operations and
2+
// hooks move here as they are extracted; until then it re-exports what the feature
3+
// already calls, so no file in the feature imports `api/` directly.
4+
export { api } from "../../api/client";
5+
export { serviceUrl } from "../../api/origin";
6+
export { pullsApi } from "../../api/pulls";
7+
export { readToken } from "../../api/token";

‎web/src/features/content/markdown/MarkdownLink.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { createContext, useContext, useEffect, useRef, useState, type MouseEvent, type ReactNode } from "react";
2-
import { api } from "../../../api/client";
2+
import { api } from "../api";
33
import { navigate, parseRoute } from "../../../product/state/route";
44
import { useUi } from "../../../product/state/ui";
55
import { useWorkspace } from "../../../product/state/workspace";

‎web/src/features/content/markdown/PullLinkPreview.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import { createPortal } from "react-dom";
1616
import type { PullRequest } from "../../../api/types";
1717
import { compactAge } from "../../../lib/format";
1818
import { pullRepositoryLabel, samePullRequest } from "./pullLink";
19-
import { pullsApi } from "../../../api/pulls";
19+
import { pullsApi } from "../api";
2020

2121
const CARD_WIDTH = 320;
2222
const CARD_HEIGHT = 150;

‎web/src/features/content/markdown/mediaSource.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { mediaKind, rawUrl } from "../../../product/media/media";
2-
import { serviceUrl } from "../../../api/origin";
3-
import { readToken } from "../../../api/token";
2+
import { serviceUrl } from "../api";
3+
import { readToken } from "../api";
44

55
/** A media element of a message, and everything the expanded view acts on. */
66
export interface MediaItem {

‎web/src/features/onboarding/DraftView/index.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { useDraftModelDefaults } from "../useDraftModelDefaults";
33
import { useDraftCheckout } from "../useDraftCheckout";
44
import { ArrowUp, Bot, Folder, GitBranch, Paperclip, X } from "lucide-react";
55
import { useEffect, useLayoutEffect, useRef, useState } from "react";
6-
import { api } from "../../../api/client";
6+
import { api } from "../api";
77
import { useCommand } from "../../../product/shell/shortcuts";
88
import type { PermissionMode, Project } from "../../../api/types";
99
import { slugify } from "../../../lib/format";

‎web/src/features/onboarding/SignIn.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { useState, type FormEvent } from "react";
2-
import { api } from "../../api/client";
2+
import { api } from "./api";
33
import { toFailure } from "../../lib/useAsync";
44
import { Button, Field } from "../../ui/controls";
55

‎web/src/features/onboarding/TrustFolder.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { useEffect, useState } from "react";
2-
import { api } from "../../api/client";
2+
import { api } from "./api";
33
import { toFailure } from "../../lib/useAsync";
44
import { navigate, routeHref, type Route } from "../../product/state/route";
55
import { useWorkspace } from "../../product/state/workspace";

‎web/src/features/onboarding/WelcomeWizard.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { Check, Copy, RefreshCw } from "lucide-react";
22
import { useEffect, useState } from "react";
3-
import { api, request } from "../../api/client";
3+
import { api, request } from "./api";
44
import { toFailure, useAsync } from "../../lib/useAsync";
55
import type { ScopedPreferencesResult } from "../../generated/ScopedPreferencesResult";
66
import type { EngineStatus, PermissionMode, Project } from "../../api/types";

0 commit comments

Comments
 (0)