fix: close remaining presentation sync races
This commit is contained in:
@@ -125,6 +125,9 @@ describe("presentation sync client", () => {
|
|||||||
body: JSON.stringify({ role: "audience", code: "ABC123" }),
|
body: JSON.stringify({ role: "audience", code: "ABC123" }),
|
||||||
}),
|
}),
|
||||||
);
|
);
|
||||||
|
expect(storage.getItem(PRESENTATION_SYNC_GRANT_STORAGE_KEY)).toBeNull();
|
||||||
|
|
||||||
|
client.connect(grant, () => {});
|
||||||
expect(storage.getItem(PRESENTATION_SYNC_GRANT_STORAGE_KEY)).toContain(
|
expect(storage.getItem(PRESENTATION_SYNC_GRANT_STORAGE_KEY)).toContain(
|
||||||
'"connectionToken":"token-1"',
|
'"connectionToken":"token-1"',
|
||||||
);
|
);
|
||||||
|
|||||||
@@ -255,7 +255,6 @@ export const createPresentationSyncClient = (
|
|||||||
|
|
||||||
const decoded = decodeSessionGrant(responseText);
|
const decoded = decodeSessionGrant(responseText);
|
||||||
if (!decoded.ok) throw new Error("server returned an invalid session grant");
|
if (!decoded.ok) throw new Error("server returned an invalid session grant");
|
||||||
saveGrant(decoded.value);
|
|
||||||
return decoded.value;
|
return decoded.value;
|
||||||
};
|
};
|
||||||
|
|
||||||
|
|||||||
@@ -171,6 +171,55 @@ describe("usePresentationSync", () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("uses committed props for stable actions and remote callbacks", async () => {
|
||||||
|
const { dependencies, sockets, fetch } = makeDependencies();
|
||||||
|
fetch.mockImplementation(async () => resolvedGrant());
|
||||||
|
const firstApply = vi.fn();
|
||||||
|
const secondApply = vi.fn();
|
||||||
|
const { result, rerender } = renderHook(
|
||||||
|
({ role, hash, applyRemoteHash }) =>
|
||||||
|
usePresentationSync({
|
||||||
|
role,
|
||||||
|
currentHash: hash,
|
||||||
|
applyRemoteHash,
|
||||||
|
dependencies,
|
||||||
|
}),
|
||||||
|
{
|
||||||
|
initialProps: {
|
||||||
|
role: "presenter" as "presenter" | "audience",
|
||||||
|
hash: "#scene/thesis/title",
|
||||||
|
applyRemoteHash: firstApply,
|
||||||
|
},
|
||||||
|
},
|
||||||
|
);
|
||||||
|
|
||||||
|
rerender({
|
||||||
|
role: "audience",
|
||||||
|
hash: "#scene/problem/direct-actions",
|
||||||
|
applyRemoteHash: secondApply,
|
||||||
|
});
|
||||||
|
|
||||||
|
await act(async () => {
|
||||||
|
await result.current.startSession();
|
||||||
|
});
|
||||||
|
expect(fetch).toHaveBeenCalledWith(
|
||||||
|
"http://console.test/api/presentation-sync/sessions",
|
||||||
|
expect.objectContaining({
|
||||||
|
body: JSON.stringify({
|
||||||
|
role: "audience",
|
||||||
|
initialHash: "#scene/problem/direct-actions",
|
||||||
|
}),
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
|
||||||
|
await act(async () => {
|
||||||
|
sockets[0]?.open();
|
||||||
|
sockets[0]?.serverMessage(snapshot("#scene/architecture/runtime", 1));
|
||||||
|
});
|
||||||
|
expect(firstApply).not.toHaveBeenCalled();
|
||||||
|
expect(secondApply).toHaveBeenCalledWith("#scene/architecture/runtime");
|
||||||
|
});
|
||||||
|
|
||||||
it("applies a remote hash without publishing it back", async () => {
|
it("applies a remote hash without publishing it back", async () => {
|
||||||
const { dependencies, sockets, fetch } = makeDependencies();
|
const { dependencies, sockets, fetch } = makeDependencies();
|
||||||
fetch.mockImplementation(async () => resolvedGrant());
|
fetch.mockImplementation(async () => resolvedGrant());
|
||||||
@@ -437,7 +486,7 @@ describe("usePresentationSync", () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
it("invalidates an in-flight operation when retry starts a newer operation", async () => {
|
it("invalidates an in-flight operation when retry starts a newer operation", async () => {
|
||||||
const { dependencies, sockets, fetch } = makeDependencies();
|
const { dependencies, sockets, storage, fetch } = makeDependencies();
|
||||||
const releases: Array<(response: Response) => void> = [];
|
const releases: Array<(response: Response) => void> = [];
|
||||||
fetch.mockImplementation(
|
fetch.mockImplementation(
|
||||||
() => new Promise<Response>((resolve) => releases.push(resolve)),
|
() => new Promise<Response>((resolve) => releases.push(resolve)),
|
||||||
@@ -464,6 +513,7 @@ describe("usePresentationSync", () => {
|
|||||||
await settleAsync();
|
await settleAsync();
|
||||||
});
|
});
|
||||||
expect(sockets).toHaveLength(0);
|
expect(sockets).toHaveLength(0);
|
||||||
|
expect(storage.getItem(PRESENTATION_SYNC_GRANT_STORAGE_KEY)).toBeNull();
|
||||||
expect(result.current.state).toMatchObject({ kind: "joining", code: "CCC333" });
|
expect(result.current.state).toMatchObject({ kind: "joining", code: "CCC333" });
|
||||||
|
|
||||||
await act(async () => {
|
await act(async () => {
|
||||||
@@ -471,18 +521,19 @@ describe("usePresentationSync", () => {
|
|||||||
await settleAsync();
|
await settleAsync();
|
||||||
});
|
});
|
||||||
expect(sockets).toHaveLength(1);
|
expect(sockets).toHaveLength(1);
|
||||||
|
expect(storage.getItem(PRESENTATION_SYNC_GRANT_STORAGE_KEY)).not.toBeNull();
|
||||||
});
|
});
|
||||||
|
|
||||||
it("ignores a deferred result after leave, end, or unmount", async () => {
|
it("ignores a deferred result after leave, end, or unmount", async () => {
|
||||||
const makeDeferredHook = () => {
|
const makeDeferredHook = (role: "presenter" | "audience" = "presenter") => {
|
||||||
const { dependencies, sockets, fetch } = makeDependencies();
|
const { dependencies, sockets, storage, fetch } = makeDependencies();
|
||||||
let resolvePending: ((response: Response) => void) | null = null;
|
let resolvePending: ((response: Response) => void) | null = null;
|
||||||
fetch.mockImplementation(
|
fetch.mockImplementation(
|
||||||
() => new Promise<Response>((resolve) => { resolvePending = resolve; }),
|
() => new Promise<Response>((resolve) => { resolvePending = resolve; }),
|
||||||
);
|
);
|
||||||
const rendered = renderHook(() =>
|
const rendered = renderHook(() =>
|
||||||
usePresentationSync({
|
usePresentationSync({
|
||||||
role: "presenter",
|
role,
|
||||||
currentHash: "#scene/thesis/title",
|
currentHash: "#scene/thesis/title",
|
||||||
applyRemoteHash: () => {},
|
applyRemoteHash: () => {},
|
||||||
dependencies,
|
dependencies,
|
||||||
@@ -492,6 +543,7 @@ describe("usePresentationSync", () => {
|
|||||||
...rendered,
|
...rendered,
|
||||||
release: (response: Response) => resolvePending?.(response),
|
release: (response: Response) => resolvePending?.(response),
|
||||||
sockets,
|
sockets,
|
||||||
|
storage,
|
||||||
};
|
};
|
||||||
};
|
};
|
||||||
|
|
||||||
@@ -500,14 +552,16 @@ describe("usePresentationSync", () => {
|
|||||||
await act(async () => { left.result.current.leaveSession(); });
|
await act(async () => { left.result.current.leaveSession(); });
|
||||||
await act(async () => { left.release(resolvedGrant()); await settleAsync(); });
|
await act(async () => { left.release(resolvedGrant()); await settleAsync(); });
|
||||||
expect(left.sockets).toHaveLength(0);
|
expect(left.sockets).toHaveLength(0);
|
||||||
|
expect(left.storage.getItem(PRESENTATION_SYNC_GRANT_STORAGE_KEY)).toBeNull();
|
||||||
expect(left.result.current.state).toEqual({ kind: "ended", reason: "left" });
|
expect(left.result.current.state).toEqual({ kind: "ended", reason: "left" });
|
||||||
left.unmount();
|
left.unmount();
|
||||||
|
|
||||||
const ended = makeDeferredHook();
|
const ended = makeDeferredHook("audience");
|
||||||
await act(async () => { void ended.result.current.startSession(); });
|
await act(async () => { void ended.result.current.joinSession("AAA111"); });
|
||||||
await act(async () => { ended.result.current.endSession(); });
|
await act(async () => { ended.result.current.endSession(); });
|
||||||
await act(async () => { ended.release(resolvedGrant()); await settleAsync(); });
|
await act(async () => { ended.release(resolvedGrant()); await settleAsync(); });
|
||||||
expect(ended.sockets).toHaveLength(0);
|
expect(ended.sockets).toHaveLength(0);
|
||||||
|
expect(ended.storage.getItem(PRESENTATION_SYNC_GRANT_STORAGE_KEY)).toBeNull();
|
||||||
expect(ended.result.current.state).toEqual({
|
expect(ended.result.current.state).toEqual({
|
||||||
kind: "ended",
|
kind: "ended",
|
||||||
reason: "presenter_ended",
|
reason: "presenter_ended",
|
||||||
@@ -593,7 +647,7 @@ describe("usePresentationSync", () => {
|
|||||||
fetch.mockImplementation(
|
fetch.mockImplementation(
|
||||||
() => new Promise<Response>((resolve) => releases.push(resolve)),
|
() => new Promise<Response>((resolve) => releases.push(resolve)),
|
||||||
);
|
);
|
||||||
const { unmount } = renderHook(
|
const { result, unmount } = renderHook(
|
||||||
() =>
|
() =>
|
||||||
usePresentationSync({
|
usePresentationSync({
|
||||||
role: "audience",
|
role: "audience",
|
||||||
@@ -604,11 +658,17 @@ describe("usePresentationSync", () => {
|
|||||||
{ reactStrictMode: true, wrapper: strictWrapper },
|
{ reactStrictMode: true, wrapper: strictWrapper },
|
||||||
);
|
);
|
||||||
|
|
||||||
expect(fetch).toHaveBeenCalledTimes(2);
|
expect(fetch).toHaveBeenCalledTimes(1);
|
||||||
|
expect(releases).toHaveLength(1);
|
||||||
releases[0]?.(resolvedGrant());
|
releases[0]?.(resolvedGrant());
|
||||||
releases[1]?.(resolvedGrant());
|
|
||||||
await act(async () => { await settleAsync(); });
|
await act(async () => { await settleAsync(); });
|
||||||
expect(sockets).toHaveLength(1);
|
expect(sockets).toHaveLength(1);
|
||||||
|
await act(async () => {
|
||||||
|
sockets[0]?.open();
|
||||||
|
sockets[0]?.serverMessage(snapshot("#scene/thesis/title", 0));
|
||||||
|
sockets[0]?.serverMessage(presence(1, 1));
|
||||||
|
});
|
||||||
|
expect(result.current.state.kind).toBe("connected");
|
||||||
unmount();
|
unmount();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
@@ -1,5 +1,6 @@
|
|||||||
import {
|
import {
|
||||||
useEffect,
|
useEffect,
|
||||||
|
useLayoutEffect,
|
||||||
useRef,
|
useRef,
|
||||||
useSyncExternalStore,
|
useSyncExternalStore,
|
||||||
} from "react";
|
} from "react";
|
||||||
@@ -53,6 +54,7 @@ type InternalController = PresentationSyncController & {
|
|||||||
readonly publish: (hash: string) => string | null;
|
readonly publish: (hash: string) => string | null;
|
||||||
readonly restoreSavedGrant: () => SessionGrant | null;
|
readonly restoreSavedGrant: () => SessionGrant | null;
|
||||||
readonly restoreGrant: (grant: SessionGrant) => void;
|
readonly restoreGrant: (grant: SessionGrant) => void;
|
||||||
|
readonly resumeJoinSession: (code: string) => void;
|
||||||
readonly browserUrl: BrowserUrlState;
|
readonly browserUrl: BrowserUrlState;
|
||||||
};
|
};
|
||||||
|
|
||||||
@@ -61,6 +63,11 @@ type LastOperation =
|
|||||||
| { readonly kind: "join"; readonly code: string }
|
| { readonly kind: "join"; readonly code: string }
|
||||||
| null;
|
| null;
|
||||||
|
|
||||||
|
type PendingJoin = {
|
||||||
|
readonly code: string;
|
||||||
|
readonly promise: Promise<SessionGrant>;
|
||||||
|
};
|
||||||
|
|
||||||
const defaultBrowserDependencies = (): {
|
const defaultBrowserDependencies = (): {
|
||||||
readonly client: PresentationSyncClientDependencies;
|
readonly client: PresentationSyncClientDependencies;
|
||||||
readonly url: BrowserUrlState;
|
readonly url: BrowserUrlState;
|
||||||
@@ -149,6 +156,7 @@ const createController = (options: {
|
|||||||
let operationGeneration = 0;
|
let operationGeneration = 0;
|
||||||
let currentGrant: SessionGrant | null = null;
|
let currentGrant: SessionGrant | null = null;
|
||||||
let lastOperation: LastOperation = null;
|
let lastOperation: LastOperation = null;
|
||||||
|
let pendingJoin: PendingJoin | null = null;
|
||||||
const listeners = new Set<() => void>();
|
const listeners = new Set<() => void>();
|
||||||
const pendingMessageIds = new Set<string>();
|
const pendingMessageIds = new Set<string>();
|
||||||
|
|
||||||
@@ -284,19 +292,33 @@ const createController = (options: {
|
|||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
const joinSession = async (code: string): Promise<void> => {
|
const joinSession = async (
|
||||||
|
code: string,
|
||||||
|
reusePending = false,
|
||||||
|
): Promise<void> => {
|
||||||
const generation = beginOperation();
|
const generation = beginOperation();
|
||||||
const normalizedCode = normalizeJoinCode(code);
|
const normalizedCode = normalizeJoinCode(code);
|
||||||
|
const reusableJoin =
|
||||||
|
reusePending && pendingJoin?.code === normalizedCode ? pendingJoin : null;
|
||||||
lastOperation = { kind: "join", code: normalizedCode };
|
lastOperation = { kind: "join", code: normalizedCode };
|
||||||
currentGrant = null;
|
currentGrant = null;
|
||||||
pendingMessageIds.clear();
|
pendingMessageIds.clear();
|
||||||
options.client.leave();
|
options.client.leave();
|
||||||
dispatch({ type: "start_join", code: normalizedCode });
|
dispatch({ type: "start_join", code: normalizedCode });
|
||||||
|
|
||||||
|
let request: Promise<SessionGrant> | null = null;
|
||||||
try {
|
try {
|
||||||
const grant = await options.client.join(options.getRole(), normalizedCode);
|
request =
|
||||||
|
reusableJoin?.promise ?? options.client.join(options.getRole(), normalizedCode);
|
||||||
|
if (reusableJoin === null) {
|
||||||
|
pendingJoin = { code: normalizedCode, promise: request };
|
||||||
|
}
|
||||||
|
const grant = await request;
|
||||||
|
if (pendingJoin?.promise === request) pendingJoin = null;
|
||||||
if (!isCurrentOperation(generation)) return;
|
if (!isCurrentOperation(generation)) return;
|
||||||
connectGrant(grant);
|
connectGrant(grant);
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
|
if (request !== null && pendingJoin?.promise === request) pendingJoin = null;
|
||||||
if (isCurrentOperation(generation)) {
|
if (isCurrentOperation(generation)) {
|
||||||
dispatch({
|
dispatch({
|
||||||
type: "failed",
|
type: "failed",
|
||||||
@@ -307,12 +329,16 @@ const createController = (options: {
|
|||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
|
const resumeJoinSession = (code: string): void => {
|
||||||
|
void joinSession(code, true);
|
||||||
|
};
|
||||||
|
|
||||||
const retry = (): void => {
|
const retry = (): void => {
|
||||||
if (lastOperation?.kind === "create") {
|
if (lastOperation?.kind === "create") {
|
||||||
void startSession();
|
void startSession();
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
if (lastOperation?.kind === "join") void joinSession(lastOperation.code);
|
if (lastOperation?.kind === "join") void joinSession(lastOperation.code, false);
|
||||||
};
|
};
|
||||||
|
|
||||||
const leaveSession = (): void => {
|
const leaveSession = (): void => {
|
||||||
@@ -364,6 +390,7 @@ const createController = (options: {
|
|||||||
publish,
|
publish,
|
||||||
restoreSavedGrant,
|
restoreSavedGrant,
|
||||||
restoreGrant,
|
restoreGrant,
|
||||||
|
resumeJoinSession,
|
||||||
browserUrl: options.browserUrl,
|
browserUrl: options.browserUrl,
|
||||||
};
|
};
|
||||||
};
|
};
|
||||||
@@ -396,9 +423,12 @@ export const usePresentationSync = ({
|
|||||||
const applyRemoteHashRef = useRef(applyRemoteHash);
|
const applyRemoteHashRef = useRef(applyRemoteHash);
|
||||||
const remoteHashInFlightRef = useRef<string | null>(null);
|
const remoteHashInFlightRef = useRef<string | null>(null);
|
||||||
const lastObservedHashRef = useRef(currentHash);
|
const lastObservedHashRef = useRef(currentHash);
|
||||||
roleRef.current = role;
|
|
||||||
currentHashRef.current = currentHash;
|
useLayoutEffect(() => {
|
||||||
applyRemoteHashRef.current = applyRemoteHash;
|
roleRef.current = role;
|
||||||
|
currentHashRef.current = currentHash;
|
||||||
|
applyRemoteHashRef.current = applyRemoteHash;
|
||||||
|
}, [applyRemoteHash, currentHash, role]);
|
||||||
|
|
||||||
const controllerRef = useRef<InternalController | null>(null);
|
const controllerRef = useRef<InternalController | null>(null);
|
||||||
const pairCodeRef = useRef<string | null>(null);
|
const pairCodeRef = useRef<string | null>(null);
|
||||||
@@ -437,7 +467,7 @@ export const usePresentationSync = ({
|
|||||||
else {
|
else {
|
||||||
const pairCode = pairCodeRef.current ?? pairCodeFromUrl(controller.browserUrl);
|
const pairCode = pairCodeRef.current ?? pairCodeFromUrl(controller.browserUrl);
|
||||||
pairCodeRef.current = pairCode;
|
pairCodeRef.current = pairCode;
|
||||||
if (pairCode !== null) void controller.joinSession(pairCode);
|
if (pairCode !== null) controller.resumeJoinSession(pairCode);
|
||||||
}
|
}
|
||||||
return () => controller.dispose();
|
return () => controller.dispose();
|
||||||
}, [controller, injectedClient]);
|
}, [controller, injectedClient]);
|
||||||
|
|||||||
Reference in New Issue
Block a user