From 61d8932eead7e079e607d5bcfddcd9a9729996e7 Mon Sep 17 00:00:00 2001 From: Sridhar Bala Date: Sun, 13 Sep 2026 02:50:26 +0530 Subject: [PATCH] fix(usePermission): ignore queries after effect cleanup --- src/usePermission/index.ts | 9 ++- src/usePermission/lifecycle.dom.test.ts | 82 +++++++++++++++++++++++++ 2 files changed, 90 insertions(+), 1 deletion(-) create mode 100644 src/usePermission/lifecycle.dom.test.ts diff --git a/src/usePermission/index.ts b/src/usePermission/index.ts index f63dec75..066f8e4c 100644 --- a/src/usePermission/index.ts +++ b/src/usePermission/index.ts @@ -5,7 +5,8 @@ import {off, on} from '../util/misc.js'; export type UsePermissionState = PermissionState | 'not-requested' | 'requested'; /** - * Tracks a permission state. + * Tracks a permission state. Query results are ignored after unmount or when + * the permission name changes. * * @param descriptor Permission request descriptor that passed to `navigator.permissions.query` */ @@ -13,6 +14,7 @@ export function usePermission(descriptor: PermissionDescriptor): UsePermissionSt const [state, setState] = useState('not-requested'); useEffect(() => { + let active = true; const unmount: RefObject<(() => void) | null> = {current: null}; setState('requested'); @@ -22,6 +24,10 @@ export function usePermission(descriptor: PermissionDescriptor): UsePermissionSt .query(descriptor) // eslint-disable-next-line promise/prefer-await-to-then,promise/always-return .then((status): void => { + if (!active) { + return; + } + const handleChange = () => { setState(status.state); }; @@ -35,6 +41,7 @@ export function usePermission(descriptor: PermissionDescriptor): UsePermissionSt }); return () => { + active = false; if (unmount.current) { unmount.current(); } diff --git a/src/usePermission/lifecycle.dom.test.ts b/src/usePermission/lifecycle.dom.test.ts new file mode 100644 index 00000000..8f7c19b6 --- /dev/null +++ b/src/usePermission/lifecycle.dom.test.ts @@ -0,0 +1,82 @@ +import {act, renderHook} from '@ver0/react-hooks-testing'; +import {afterEach, beforeEach, describe, expect, it, vi} from 'vitest'; +import {usePermission} from './index.js'; +import {expectResultValue} from '../util/testing/test-helpers.js'; + +function pendingQuery() { + let resolveQuery!: (status: PermissionStatus) => void; + const promise = new Promise((resolve) => { + resolveQuery = resolve; + }); + return {promise, resolve: resolveQuery}; +} + +function permissionStatus(state: PermissionState): PermissionStatus { + return Object.assign(new EventTarget(), {state, name: 'geolocation', onchange: null}); +} + +describe('usePermission query lifecycle', () => { + const query = vi.fn<(descriptor: PermissionDescriptor) => Promise>(); + + beforeEach(() => { + query.mockReset(); + vi.stubGlobal('navigator', {permissions: {query}}); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it('does not subscribe when a query resolves after unmount', async () => { + const pending = pendingQuery(); + const status = permissionStatus('granted'); + const subscribe = vi.spyOn(status, 'addEventListener'); + query.mockReturnValue(pending.promise); + const {unmount} = await renderHook(() => usePermission({name: 'geolocation'})); + + await unmount(); + await act(async () => { + pending.resolve(status); + }); + + expect(subscribe).not.toHaveBeenCalled(); + }); + + it('ignores an older query that resolves after the descriptor changes', async () => { + const oldQuery = pendingQuery(); + const newQuery = pendingQuery(); + const oldStatus = permissionStatus('denied'); + const newStatus = permissionStatus('granted'); + const subscribe = vi.spyOn(oldStatus, 'addEventListener'); + query.mockReturnValueOnce(oldQuery.promise).mockReturnValueOnce(newQuery.promise); + let name: PermissionName = 'geolocation'; + const {result, rerender} = await renderHook(() => usePermission({name})); + + name = 'notifications'; + await rerender(); + await act(async () => { + newQuery.resolve(newStatus); + }); + expect(expectResultValue(result)).toBe('granted'); + await act(async () => { + oldQuery.resolve(oldStatus); + }); + + expect(expectResultValue(result)).toBe('granted'); + expect(subscribe).not.toHaveBeenCalled(); + }); + + it('removes a resolved query listener on unmount', async () => { + const status = permissionStatus('prompt'); + const subscribe = vi.spyOn(status, 'addEventListener'); + const unsubscribe = vi.spyOn(status, 'removeEventListener'); + query.mockResolvedValue(status); + const {result, unmount} = await renderHook(() => usePermission({name: 'geolocation'})); + expect(expectResultValue(result)).toBe('prompt'); + expect(subscribe).toHaveBeenCalledOnce(); + + await unmount(); + + expect(unsubscribe).toHaveBeenCalledOnce(); + }); +});