Skip to content

ref(hooks): Replace HookStore with a plain hook registry#115811

Merged
evanpurkhiser merged 1 commit into
masterfrom
evanpurkhiser/ref-hooks-replace-hookstore-with-a-plain-hook-registry
May 19, 2026
Merged

ref(hooks): Replace HookStore with a plain hook registry#115811
evanpurkhiser merged 1 commit into
masterfrom
evanpurkhiser/ref-hooks-replace-hookstore-with-a-plain-hook-registry

Conversation

@evanpurkhiser
Copy link
Copy Markdown
Member

@evanpurkhiser evanpurkhiser commented May 19, 2026

Introduce static/app/hookRegistry.tsx — a simple Map-backed registry with registerHook() and getHook() exports — and delete hookStore.tsx entirely.

What changed:

  • HookStore.get() / HookStore.set()getHook() / registerHook() across all call sites
  • Hook and HookOrDefault components simplified to plain render-time reads — no useState, no useEffect, no Reflux listen
  • HookStore.persistCallback / getCallback replaced by a plain module-level cell in setOrganizationCallback.tsx
  • hookStore.tsx deleted; HookStore removed from the SentryApp browser globals
  • All test files updated to use registerHook / getHook; HookStore.init() cleanup calls removed

@evanpurkhiser evanpurkhiser requested review from a team as code owners May 19, 2026 16:22
@evanpurkhiser evanpurkhiser requested review from scttcper and removed request for a team May 19, 2026 16:22
@github-actions github-actions Bot added the Scope: Frontend Automatically applied to PRs that change frontend components label May 19, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 19, 2026

📊 Type Coverage Diff

Metric Before After Delta
Coverage 93.56% 93.56% ±0%
Typed 135,547 135,521 🔴 -26
Untyped 9,334 9,335 🔴 +1
🔍 2 new type safety issues introduced

any-typed symbols (1 new)

File Line Detail
static/app/hookRegistry.tsx 12 registry (var)

Type assertions (as) (1 new)

File Line Detail
static/app/hookRegistry.tsx 26 `as Hooks[H]

This is informational only and does not block the PR.

@evanpurkhiser evanpurkhiser force-pushed the evanpurkhiser/ref-hooks-replace-hookstore-with-a-plain-hook-registry branch from 9b265a7 to 92ade77 Compare May 19, 2026 16:31
@evanpurkhiser evanpurkhiser force-pushed the evanpurkhiser/ref-hooks-replace-hookstore-with-a-plain-hook-registry branch 2 times, most recently from f55f531 to 3fb7591 Compare May 19, 2026 16:34
Introduce static/app/hookRegistry.tsx — a simple Map-backed registry
with registerHook() and getHook() exports. Replace all HookStore.get()
and HookStore.set() call sites with the new functions.

Hook and HookOrDefault components are simplified to plain render-time
reads with no useState/useEffect/listen overhead.

HookStore's Reflux machinery (trigger, listen, remove) is no longer
used by any hook consumer.
@evanpurkhiser evanpurkhiser force-pushed the evanpurkhiser/ref-hooks-replace-hookstore-with-a-plain-hook-registry branch from 3fb7591 to 74c586c Compare May 19, 2026 16:35
@evanpurkhiser evanpurkhiser enabled auto-merge (squash) May 19, 2026 16:38
@evanpurkhiser evanpurkhiser merged commit 49f9204 into master May 19, 2026
70 checks passed
@evanpurkhiser evanpurkhiser deleted the evanpurkhiser/ref-hooks-replace-hookstore-with-a-plain-hook-registry branch May 19, 2026 16:41
Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 74c586c. Configure here.

beforeEach(() => {
HookStore.init();
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing registry cleanup causes test order dependency

Low Severity

The beforeEach(() => { HookStore.init(); }) cleanup was removed, but hookRegistry.tsx exposes no way to clear or remove registered hooks. The first test ("should render default") relies on component:replay-onboarding-cta not being registered, while the second test registers it. Since the module-level Map persists across tests within a file, any reordering (or future test insertion) that places a registerHook call before this test will cause it to fail. A clearHooks() (or per-key removeHook()) export is needed for test isolation.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 74c586c. Configure here.

evanpurkhiser added a commit that referenced this pull request May 19, 2026
Follow-up to #115811, which introduced the override registry but kept
the old "hook" terminology on the public-facing APIs. This change
finishes the rename so the codebase consistently uses "override".

## What changed

- `hookRegistry.tsx` is replaced by `overrideRegistry.tsx`;
`registerHook` / `getHook` become `registerOverride` / `getOverride`.
- `types/hooks.tsx` moves to `types/overrides.tsx`; `HookName`, `Hooks`,
and the various `*Hook*` type aliases (e.g. `FeatureDisabledHooks`) are
renamed to their `Override` equivalents.
- The `Hook` component (`components/hook.tsx`) is replaced by `Override`
(`components/override.tsx`). It is now a named export (`export
{Override}`) rather than a default export, and the `memo()` wrapper has
been dropped — the implementation is a single registry lookup, so
memoization wasn't buying anything.
- `HookOrDefault` is renamed to `OverrideOrDefault` (file, factory
function, and its `hookName` parameter → `overrideName`).
- `<Feature hookName=...>` is renamed to `<Feature overrideName=...>`
and all call sites updated.
- `gsApp/registerHooks.tsx` is renamed to `gsApp/registerOverrides.tsx`.
- `gsApp/hooks/` is renamed to `gsApp/overrides/` and contains only
override implementations now. The eleven pure React hooks that were
previously mixed in (`useAM2ProfilingUpsellModal`, `useAM2UpsellModal`,
`useBillingConfig`, `useBillingDetails`, `useCurrentBillingHistory`,
`useIntentData`, `usePlanMigrations`, `useProductBillingMetadata`,
`useRecurringCredits`, `useStripeInstance`, `useSubscription`) move to a
new `gsApp/hooks/` directory.
- The `no-calling-components-as-functions` ESLint rule's allowlist is
updated from `HookOrDefault` to `OverrideOrDefault` (it's a factory, not
a component).
- Tests, JSDoc, and inline comments throughout are updated to match.

This is a pure rename — no behavior changes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants