Skip to content

Commit 19fbbf4

Browse files
pallyoungpallyoung
committed
fix(web): persist explicit workspace target switches
1 parent 7fb53b4 commit 19fbbf4

4 files changed

Lines changed: 150 additions & 4 deletions

File tree

‎packages/web/src/features/topbar/components/tab.tsx‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,9 @@ export const WorkspaceTab: FC<WorkspaceTabProps> = ({ workspace, isActive }) =>
3434
const selectWorkspaceTarget = useSelectWorkspaceTarget();
3535
const displayName = formatWorkspaceLabel(workspace) || workspace.id;
3636

37-
const handleClick = () => {
37+
const handleClick = (e: React.MouseEvent<HTMLButtonElement>) => {
38+
e.preventDefault();
39+
3840
if (isActive) {
3941
return;
4042
}

‎packages/web/src/features/topbar/index.test.tsx‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,40 @@ describe("TopBar", () => {
122122
});
123123
});
124124

125+
it("persists the global last-viewed target once when clicking a workspace tab", async () => {
126+
const store = createStore();
127+
const sendCommand = vi.fn().mockResolvedValue({
128+
workspaceId: "ws-b",
129+
updatedAt: 10,
130+
});
131+
132+
store.set(localeAtom, "en");
133+
store.set(wsClientAtom, { sendCommand } as never);
134+
store.set(workspacesAtom, {
135+
"ws-a": createWorkspace("ws-a", "/tmp/a"),
136+
"ws-b": createWorkspace("ws-b", "/tmp/b"),
137+
});
138+
store.set(workspaceOrderAtom, ["ws-a", "ws-b"]);
139+
store.set(workspacesLoadStateAtom, "ready");
140+
store.set(activeWorkspaceIdAtom, "ws-a");
141+
142+
render(
143+
<Provider store={store}>
144+
<TopBar />
145+
</Provider>
146+
);
147+
148+
fireEvent.click(screen.getByRole("tab", { name: "b" }));
149+
150+
await waitFor(() => {
151+
expect(sendCommand).toHaveBeenCalledTimes(1);
152+
expect(sendCommand).toHaveBeenCalledWith(
153+
"workspace.lastViewedTarget.set",
154+
{ workspaceId: "ws-b", sessionId: undefined },
155+
undefined
156+
);
157+
});
158+
});
125159
it("uses translated labels when locale is set to en", () => {
126160
const store = createStore();
127161
store.set(localeAtom, "en");

‎packages/web/src/features/workspace/actions/use-persist-workspace-last-viewed-target.test.tsx‎

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,20 @@ function wrapperFor(store: ReturnType<typeof createStore>) {
1313
};
1414
}
1515

16+
function createDeferred<T>() {
17+
let resolve!: (value: T) => void;
18+
let reject!: (error: unknown) => void;
19+
const promise = new Promise<T>((nextResolve, nextReject) => {
20+
resolve = nextResolve;
21+
reject = nextReject;
22+
});
23+
24+
return {
25+
promise,
26+
resolve,
27+
reject,
28+
};
29+
}
1630
describe("usePersistWorkspaceLastViewedTarget", () => {
1731
it("does not suppress a retry for the same target after a failed write", async () => {
1832
const store = createStore();
@@ -60,4 +74,94 @@ describe("usePersistWorkspaceLastViewedTarget", () => {
6074
undefined
6175
);
6276
});
77+
it("does not roll back a newer target when an older write fails out of order", async () => {
78+
const store = createStore();
79+
const firstWrite = createDeferred<{ workspaceId: string; updatedAt: number }>();
80+
const secondWrite = createDeferred<{ workspaceId: string; updatedAt: number }>();
81+
const sendCommand = vi
82+
.fn()
83+
.mockImplementationOnce(() => firstWrite.promise)
84+
.mockImplementationOnce(() => secondWrite.promise);
85+
86+
store.set(wsClientAtom, {
87+
sendCommand,
88+
subscribe: vi.fn(() => () => {}),
89+
} as never);
90+
store.set(lastViewedTargetAtom, null);
91+
92+
const { result } = renderHook(() => usePersistWorkspaceLastViewedTarget(), {
93+
wrapper: wrapperFor(store),
94+
});
95+
96+
let firstRequest!: Promise<unknown>;
97+
let secondRequest!: Promise<unknown>;
98+
99+
act(() => {
100+
firstRequest = result.current({ workspaceId: "ws-1" });
101+
secondRequest = result.current({ workspaceId: "ws-2" });
102+
});
103+
104+
expect(store.get(lastViewedTargetAtom)?.workspaceId).toBe("ws-2");
105+
106+
await act(async () => {
107+
secondWrite.resolve({ workspaceId: "ws-2", updatedAt: 22 });
108+
await secondRequest;
109+
});
110+
111+
expect(store.get(lastViewedTargetAtom)?.workspaceId).toBe("ws-2");
112+
113+
await act(async () => {
114+
firstWrite.reject(
115+
new CommandResultError({
116+
code: "write_failed",
117+
message: "failed",
118+
})
119+
);
120+
await firstRequest;
121+
});
122+
123+
expect(store.get(lastViewedTargetAtom)?.workspaceId).toBe("ws-2");
124+
});
125+
126+
it("does not overwrite a newer target when an older write succeeds out of order", async () => {
127+
const store = createStore();
128+
const firstWrite = createDeferred<{ workspaceId: string; updatedAt: number }>();
129+
const secondWrite = createDeferred<{ workspaceId: string; updatedAt: number }>();
130+
const sendCommand = vi
131+
.fn()
132+
.mockImplementationOnce(() => firstWrite.promise)
133+
.mockImplementationOnce(() => secondWrite.promise);
134+
135+
store.set(wsClientAtom, {
136+
sendCommand,
137+
subscribe: vi.fn(() => () => {}),
138+
} as never);
139+
store.set(lastViewedTargetAtom, null);
140+
141+
const { result } = renderHook(() => usePersistWorkspaceLastViewedTarget(), {
142+
wrapper: wrapperFor(store),
143+
});
144+
145+
let firstRequest!: Promise<unknown>;
146+
let secondRequest!: Promise<unknown>;
147+
148+
act(() => {
149+
firstRequest = result.current({ workspaceId: "ws-1" });
150+
secondRequest = result.current({ workspaceId: "ws-2" });
151+
});
152+
153+
await act(async () => {
154+
secondWrite.resolve({ workspaceId: "ws-2", updatedAt: 22 });
155+
await secondRequest;
156+
});
157+
158+
expect(store.get(lastViewedTargetAtom)?.workspaceId).toBe("ws-2");
159+
160+
await act(async () => {
161+
firstWrite.resolve({ workspaceId: "ws-1", updatedAt: 11 });
162+
await firstRequest;
163+
});
164+
165+
expect(store.get(lastViewedTargetAtom)?.workspaceId).toBe("ws-2");
166+
});
63167
});

‎packages/web/src/features/workspace/actions/use-persist-workspace-last-viewed-target.ts‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import type { WorkspaceLastViewedTarget } from "@coder-studio/core";
2-
import { useAtomValue, useSetAtom } from "jotai";
2+
import { useAtomValue, useSetAtom, useStore } from "jotai";
33
import { useCallback } from "react";
44
import { lastViewedTargetAtom } from "../../../atoms/app-ui";
55
import { dispatchCommandAtom } from "../../../atoms/connection";
@@ -11,11 +11,13 @@ interface PersistWorkspaceLastViewedTargetInput {
1111

1212
export function usePersistWorkspaceLastViewedTarget() {
1313
const dispatch = useAtomValue(dispatchCommandAtom);
14-
const lastViewedTarget = useAtomValue(lastViewedTargetAtom);
1514
const setLastViewedTarget = useSetAtom(lastViewedTargetAtom);
15+
const store = useStore();
1616

1717
return useCallback(
1818
async ({ workspaceId, sessionId }: PersistWorkspaceLastViewedTargetInput) => {
19+
const lastViewedTarget = store.get(lastViewedTargetAtom);
20+
1921
if (!workspaceId) {
2022
return null;
2123
}
@@ -40,6 +42,10 @@ export function usePersistWorkspaceLastViewedTarget() {
4042
sessionId,
4143
});
4244

45+
if (store.get(lastViewedTargetAtom) !== optimisticTarget) {
46+
return store.get(lastViewedTargetAtom);
47+
}
48+
4349
if (!result.ok || !result.data) {
4450
setLastViewedTarget(previousTarget);
4551
return optimisticTarget;
@@ -48,6 +54,6 @@ export function usePersistWorkspaceLastViewedTarget() {
4854
setLastViewedTarget(result.data);
4955
return result.data;
5056
},
51-
[dispatch, lastViewedTarget, setLastViewedTarget]
57+
[dispatch, setLastViewedTarget, store]
5258
);
5359
}

0 commit comments

Comments
 (0)