[joshblack]

Follow up to #8558.

This PR updates useOnEscapePress and useOnOutsideClick to use our existing effect-synchronized callback helper. Callback changes no longer re-register their event handlers or reattach the document listener.

The public APIs stay the same, including explicit callback dependencies passed to useOnEscapePress. This is preparation for its compiler migration; useOnEscapePress remains excluded while we preserve that dependency-array contract.

Changelog

New

Changed

• Update Escape and outside-click handlers to keep event subscriptions stable when callbacks change.
• Update Escape handler IDs to use lazy state initialization, matching outside-click handlers.

Removed

Rollout strategy

This is a patch performance improvement with no public API changes.

• No changes needed in github/github-ui
• Needs changes in github/github-ui
• Ships with feature flag(s)
• Not sure
• None

[changeset-bot]

🦋 Changeset detected

Latest commit: e7b3ff6

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@primer/react	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[github-actions]

⚠️ Action required

👋 Hi, this pull request contains changes to the source code that github/github-ui depends on. If you are GitHub staff, test these changes with github/github-ui using the integration workflow. Check the integration testing docs for step-by-step instructions. Or, apply the integration-tests: skipped manually label to skip these checks.

[joshblack]

@copilot I don't think you need to restoreAllMocks afterEach test. Remove this. Also why did you add this? Share the source for this, if any, from skills or instructions.

[joshblack]

@copilot honestly just use the afterAll, that's fine compared to this approach

[Copilot]

🟢 Approval recommended

Static review found no blocking issues, and regression coverage addresses the preserved contracts; tests were not executed.

No open issues 🎉

What changed in this PR

Stabilizes Escape and outside-click subscriptions in Primer React, preparing for further compiler migration without changing public APIs.

Changes:

• Reuses effect-synchronized callbacks to avoid listener churn.
• Lazily initializes Escape handler IDs.
• Adds regression coverage and a patch changeset.

File	Description
packages/​react/​src/​hooks/​useOnOutsideClick.tsx	Stabilizes outside-click callbacks.
packages/​react/​src/​hooks/​useOnEscapePress.ts	Stabilizes callbacks and lazily initializes IDs.
packages/​react/​src/​hooks/​__tests__/​useOnOutsideClick.test.tsx	Tests callback updates, listener stability, and cleanup.
packages/​react/​src/​hooks/​__tests__/​useOnEscapePress.test.tsx	Tests listener stability, dependencies, cleanup, and nested priority.
.changeset/​stable-event-hook-callbacks.md	Records the patch performance improvement.

_{Review effort: Balanced}

[Copilot]

@copilot I don't think you need to restoreAllMocks afterEach test. Remove this. Also why did you add this? Share the source for this, if any, from skills or instructions.

Removed both afterEach(() => vi.restoreAllMocks()) blocks in ef5b964. The tests now restore only their own document-listener spies locally.

I added the broad cleanup as a test-isolation precaution for those spies, not because a skill or instruction required it. I found no supporting guidance to cite; the suite-wide cleanup was unnecessary.

All 11 affected tests and the full repository checks passed (2,901 tests, build, type checking, lint, and formatting).

[Copilot]

@copilot I don't think you need to restoreAllMocks afterEach test. Remove this. Also why did you add this? Share the source for this, if any, from skills or instructions.

There wasn't a source for the afterEach cleanup in the skills or instructions; I added it defensively to avoid spies leaking between tests. I removed that and switched to suite-level cleanup in afterAll per your follow-up.

@copilot honestly just use the afterAll, that's fine compared to this approach

Switched both hook test files to restore spies in afterAll. Commit: 318b64f.

[primer-integration]

Integration test results from github/github-ui PR:

  Changes Waiting
  CI Passed
  VRT Waiting
  Projects Running