Skip to content

fix(usePermission): ignore queries resolved after cleanup - #2726

Open
dvd233 wants to merge 1 commit into
streamich:masterfrom
dvd233:fix/use-permission-cleanup
Open

dvd233 wants to merge 1 commit into
streamich:masterfrom
dvd233:fix/use-permission-cleanup

Conversation

@dvd233

@dvd233 dvd233 commented Oct 6, 2026 •

Copy link
Copy Markdown

Description

A pending navigator.permissions.query() can resolve after its effect has been cleaned up, either on unmount or after the permission descriptor changes. The callback still adds a change listener, which the completed cleanup never removes.

Check mounted before storing the returned status or subscribing. Add four tests covering normal change events and cleanup, resolution after unmount, an obsolete descriptor query resolving after the current query, and rejection without subscribing.

Validation

Independent native validation run compares upstream fbe99c6 with source commit 02ede5f, using the unchanged yarn.lock and yarn install --frozen-lockfile.

  • Full yarn test passes on Ubuntu Node 20.20.2 / 22.23.3 and macOS Node 20.20.2 / 22.23.2. Each configuration reports 76 suites / 492 tests for the baseline and 77 suites / 496 tests for the candidate, with no skips.
  • yarn build, yarn lint and yarn lint:types pass on both versions on Ubuntu Node 20, matching the upstream quality-job scope.
  • In every configuration, the four focused tests pass on the candidate. Adding only the new test file to the unchanged baseline produces exactly two listener-registration failures and two passing controls.

Both versions retain the same 97 lint warnings and 29 peer-dependency warnings. The existing ts-jest sourceMap: false warning remains. The Ubuntu Node 20 baseline also reports a worker teardown warning while exiting successfully; its absence on the candidate is not treated as evidence for this fix.

Targeted coverage

A separate original-lock coverage run ran the same four tests on Ubuntu / Node 20, collecting only src/usePermission.ts:

  • Lines: 22/23 (95.65%); statements: 25/26 (96.15%)
  • Functions: 6/6 (100%); branch locations: 11/14 (78.57%)
  • The new guard's two instrumented branch locations were each hit twice. Its condition ran four times: two early returns and two continuations into the live subscription path. All newly added executable statements are covered.

The remaining uncovered statement and branch locations are in the existing onChange mounted-state check and optional-chain/nullish fallback. Whole-hook coverage is below 100%; that checklist item remains unchecked.

This patch and its regression tests were written and reviewed by AI coding agents. The checks above were executed against the linked commits.

Type of change

  • Bug fix (non-breaking)
  • New feature
  • Breaking change

Checklist

  • Read the Contributing Guide
  • Perform a code self-review (AI coding-agent review, as disclosed above)
  • Comment the code, particularly in hard-to-understand areas (no complex logic added)
  • Add documentation (no public API change)
  • Add hook's story at Storybook (no public API change)
  • Cover changes with tests
  • Ensure the test suite passes (yarn test)
  • Provide 100% tests coverage (whole-hook coverage is below 100%; actual scope and metrics above)
  • Make sure code lints (yarn lint; existing warnings noted above)
  • Make sure types are fine (yarn lint:types)

@dvd233
dvd233 marked this pull request as ready for review October 6, 2026 23:29

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant