Skip to content

Commit 84959eb

Browse files
Fix stale lens removal after cancelled apply (#9)
## Summary - Stop `useApplyLens` from calling `removeLens()` again when a cancelled `applyLens()` resolves later - Keep cleanup-time `removeLens()` intact for unmounts / lens intent changes - Add regression coverage for a superseded lens apply resolving after a newer lens has started ## Why Camera Kit core already handles apply/replace ordering. The React wrapper was issuing a second stale `removeLens()` after an old cancelled apply resolved, which could clear the newer active lens during fast lens switching. ## Test Plan - `npm test -- useApplyLens.test.ts --runInBand` - `npm run typecheck` - `npm run format:check` - `npm test -- --runInBand`
1 parent c139760 commit 84959eb

4 files changed

Lines changed: 48 additions & 18 deletions

File tree

package-lock.json

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@snap/react-camera-kit",
3-
"version": "0.3.0",
3+
"version": "0.3.1",
44
"description": "React Camera Kit for web applications",
55
"type": "module",
66
"main": "./dist/cjs/index.js",

src/useApplyLens.test.ts

Lines changed: 44 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { renderHook, waitFor } from "@testing-library/react";
1+
import { act, renderHook, waitFor } from "@testing-library/react";
22
import hash from "stable-hash";
33
import { useApplyLens } from "./useApplyLens";
44
import { useInternalCameraKit } from "./CameraKitProvider";
@@ -15,6 +15,16 @@ const mockUseInternalCameraKit = useInternalCameraKit as jest.MockedFunction<typ
1515
const mockHash = hash as jest.MockedFunction<typeof hash>;
1616
const mockReportCount = metricsReporter.reportCount as jest.Mock;
1717

18+
function deferred<T>() {
19+
let resolve: (value: T) => void = () => {};
20+
let reject: (reason?: unknown) => void = () => {};
21+
const promise = new Promise<T>((res, rej) => {
22+
resolve = res;
23+
reject = rej;
24+
});
25+
return { promise, resolve, reject };
26+
}
27+
1828
describe("useApplyLens", () => {
1929
let mockApplyLens: jest.Mock;
2030
let mockRemoveLens: jest.Mock;
@@ -387,14 +397,9 @@ describe("useApplyLens", () => {
387397
});
388398

389399
describe("Cancellation handling", () => {
390-
it("should remove lens if component unmounts during application", async () => {
391-
let resolveApply: any;
392-
mockApplyLens.mockImplementation(
393-
() =>
394-
new Promise((resolve) => {
395-
resolveApply = resolve;
396-
}),
397-
);
400+
it("should not remove lens again if component unmounts during application", async () => {
401+
const apply = deferred<boolean>();
402+
mockApplyLens.mockReturnValue(apply.promise);
398403

399404
const { unmount } = renderHook(() => useApplyLens("lens-123", "group-456"));
400405

@@ -403,11 +408,39 @@ describe("useApplyLens", () => {
403408
});
404409

405410
unmount();
406-
resolveApply(true);
411+
await act(async () => {
412+
apply.resolve(true);
413+
await apply.promise;
414+
});
415+
416+
expect(mockRemoveLens).toHaveBeenCalledTimes(1);
417+
});
418+
419+
it("should not remove the current lens when a superseded apply resolves late", async () => {
420+
const firstApply = deferred<boolean>();
421+
mockApplyLens.mockImplementationOnce(() => firstApply.promise).mockResolvedValueOnce(true);
422+
423+
const { rerender } = renderHook(({ lensId }) => useApplyLens(lensId, "group-1"), {
424+
initialProps: { lensId: "lens-1" },
425+
});
426+
427+
await waitFor(() => {
428+
expect(mockApplyLens).toHaveBeenCalledWith("lens-1", "group-1", undefined, undefined);
429+
});
430+
431+
rerender({ lensId: "lens-2" });
407432

408433
await waitFor(() => {
409-
expect(mockRemoveLens).toHaveBeenCalledTimes(2); // Once after apply, once on unmount
434+
expect(mockApplyLens).toHaveBeenCalledWith("lens-2", "group-1", undefined, undefined);
410435
});
436+
expect(mockRemoveLens).toHaveBeenCalledTimes(1);
437+
438+
await act(async () => {
439+
firstApply.resolve(true);
440+
await firstApply.promise;
441+
});
442+
443+
expect(mockRemoveLens).toHaveBeenCalledTimes(1);
411444
});
412445
});
413446

src/useApplyLens.ts

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -71,10 +71,7 @@ export function useApplyLens(
7171
(async () => {
7272
try {
7373
await applyLens(lensId, lensGroupId, safeLaunchData, guardRef.current);
74-
if (cancelled) {
75-
await removeLens();
76-
return;
77-
}
74+
if (cancelled) return;
7875
log.info("apply_success", {
7976
lensId,
8077
groupId: lensGroupId,

0 commit comments

Comments
 (0)