Simplify and Focus Invite Members Callout Code

Summary

I found it: that branch is checked out in /Users/maxgeorgopoulos/Documents/traces-worktrees/codex-feat-onboarding-team-step (branch name currently spelled codex/feat-invite-memebers-callout). I’ll switch context there now.

Ran

You’re right to call it out. It’s there because MembersList currently needs an imperative reload function, not just passive data.

In MembersList.tsx, fetchMembers is reused:
- on mount (useEffect)
- after PATCH role changes
- after DELETE member removal
In useNamespaceMembers.ts, the hook only returns { members, isMember, isAdmin }.
It does not expose loading, error, or refetch.

So yes, this is duplicated fetch orchestration right now. The clean fix is to make useNamespaceMembers return:

members
isMember
isAdmin
loading
error
refetch()

Then MembersList can drop that local useCallback + useEffect block and call refetch() after mutations.

when should you use a useCallback?

Use useCallback only when function identity matters.

Good reasons:

You pass the function to a memoized child (memo) and want to avoid needless re-renders.
The function is in a hook dependency array and would otherwise recreate/retrigger expensive effects/subscriptions.
You need a stable function reference for add/remove listener symmetry.

Don’t use it:

For ordinary inline handlers in the same component with no memoization pressure.
Just “for performance” by default.
To silence lints without understanding dependencies.

In your MembersList case, useCallback is mostly there because useEffect depends on fetchMembers. If we move to a hook that exposes refetch, we can avoid this pattern entirely.

gotcha, let's do this then:

So yes, this is duplicated fetch orchestration right now. The clean fix is to make useNamespaceMembers return:

members
isMember
isAdmin
loading
error
refetch()
Then MembersList can drop that local useCallback + useEffect block and call refetch() after mutations.

Ran

I’m going to refactor useNamespaceMembers to expose loading, error, and refetch, then wire MembersList to consume the hook directly and remove its local fetch useCallback/useEffect.

Ran

I’ve wired MembersList to the hook and removed its local fetch effect/callback. I’m running typecheck now to catch any typing or lint regressions from this refactor.

Ran

Implemented.

Changes made:

Updated useNamespaceMembers.ts

Added return fields: loading, error, refetch()
Kept existing members, isMember, isAdmin
Added request de-dupe/cancellation guard with request ids
refetch() now drives member reloads and is used by the internal load effect

Updated MembersList.tsx

Removed local fetch orchestration (useCallback + useEffect)
Switched to useNamespaceMembers({ slug, shouldFetch: true, userId })
After role change / remove actions, calls await refetch()
Added local actionError and merged display with hook error
Prevented full skeleton flicker on refetch by only showing skeleton when loading && members.length === 0

Validation:

Ran bun run typecheck successfully.

Error: Calling setState synchronously within an effect can trigger cascading renders

Effects are intended to synchronize state between React and external systems such as manually updating the DOM, state management libraries, or other platform APIs. In general, the body of an effect should do one or both of the following:

Update external systems with the latest state from React.
Subscribe for updates from some external system, calling setState in a callback function when external state changes.

Calling setState synchronously within an effect body causes cascading renders that can hurt performance, and is not recommended. (https://react.dev/learn/you-might-not-need-an-effect).

/Users/maxgeorgopoulos/Documents/traces-worktrees/codex-feat-onboarding-team-step/frontend/hooks/useNamespaceMembers.ts:130:10
128 | }
129 |

130 | void refetch();
| ^^^^^^^ Avoid calling setState() directly within an effect
131 | }, [shouldFetch, refetch]);
132 |
133 | const isResolvedForSlug = resolvedSlug === slug;eslint(react-hooks/set-state-in-effect)

I see the lint cause: the effect invokes refetch(), and refetch sets state before its first await. I’ll patch the hook so the effect does its own async load path (no sync setState in effect) while keeping refetch() for button/action refreshes.

Ran

I’m patching just the effect: it will fetch directly with an async IIFE (no refetch() call inside the effect), which removes the lint issue while keeping refetch() for manual refresh after mutations.

Ran

I fixed the pattern that triggered that lint rule. I’m running ESLint on the hook file directly to confirm the warning is gone, not just typecheck.

Ran

I confirmed the branch tracks origin/codex/feat-invite-memebers-callout. Next I’ll inspect existing commit trailers in this repo so the co-author line matches your project’s expected format exactly.

Ran

Fixed.

I removed the void refetch() call from the effect in useNamespaceMembers.ts and replaced it with an inline async load in the effect that only sets state after await fetchNamespaceMembers(...).

refetch() is still exposed/used for manual refreshes (like post-mutation in MembersList.tsx), but the effect no longer invokes a function that performs synchronous state updates.

Validation:

bun run typecheck passed.
Targeted lint passed for the hook: ./node_modules/.bin/eslint hooks/useNamespaceMembers.ts (run in frontend/).

most of the usNamespacememebrs code for fetching is duplicated in refetch

You’re right. I’m going to extract the shared “apply fetch result” path so refetch and the effect only differ in trigger behavior, not full logic.

Ran

I extracted the shared fetch-result handling into one callback and rewired both paths to use it. I’ll run typecheck and targeted lint again to ensure the duplicate cleanup didn’t reintroduce the effect warning.

Ran

Agreed, and fixed.

I deduplicated useNamespaceMembers by extracting shared result handling into one function:

Added applyFetchResult(requestId, slug, result) in
useNamespaceMembers.ts
Both the initial effect load and refetch() now call that helper.
This removes the repeated success/error state-application logic from both paths.

Validation:

bun run typecheck passed.
Targeted lint on the hook passed:
- frontend/. → ./node_modules/.bin/eslint hooks/useNamespaceMembers.ts

for frontend/lib/invite-callout-storage.ts, what's the point of having one things for completed and another for dismissed?

I’ll open that storage helper so I can answer based on the exact behavior it currently implements.

Ran

I’m also checking the callout component usage, because the value of two flags depends on how each one is interpreted there.

Ran

Great question. In principle:

dismissed means user explicitly hid it (X / “Maybe later”).
completed means goal achieved (invite flow done), so it should auto-hide.

In your code, that maps to:

markInviteCalloutDismissed in InviteMembersCallout.tsx
markInviteCalloutCompleted in InviteManager.tsx (and also member-count logic in callout)

But today, both flags are effectively used the same way for visibility (hide if either true), so functionally they’re mostly semantic separation right now. If you don’t need that distinction for analytics or future re-show rules, we can collapse to a single hidden/closed key.

yeah let's merge it into just "closed"

I’m staging only the three modified frontend files, then I’ll commit with a Co-Authored-By: GPT-5 Codex <codex@openai.com> trailer and push to the tracked branch.

I’ll collapse the callout storage to a single closed key and update both InviteMembersCallout and InviteManager to use that shared state. Then I’ll run typecheck.

Ran

I’m editing three files now: storage helper, callout, and invite manager. I’ll keep backward compatibility by treating old dismissed/completed keys as closed on read so existing users stay closed.

Ran

I’ve applied the closed merge and updated callsites. I’m running typecheck now to verify no stale references remain.

Ran

There’s one lint error: setIsClosed(true) inside the effect. I’ll remove that state write and rely on memberCount > 1 for immediate hiding while still persisting closed to storage.

Ran

Done. I merged invite callout state into a single closed flag.

Updated:

invite-callout-storage.ts
- Replaced isDismissed + isCompleted with isClosed
- Replaced separate keys with one key: traces:invite-callout:closed:<slug>
- Added backward compatibility on read: old dismissed/completed keys still count as closed
InviteMembersCallout.tsx
- Uses isClosed + markInviteCalloutClosed
- Close button and “Maybe later” both set closed
- Auto-close persistence when member count > 1 now writes storage only (no setState-in-effect lint issue)
InviteManager.tsx
- On invite creation, now calls markInviteCalloutClosed

Validation:

Targeted ESLint passed for changed frontend files.
bun run typecheck passed.

don't keep backwards compatability with the keys

I’ll remove the legacy key fallback and make the callout read/write only the new closed key. Then I’ll run a quick check.

Ran

Done. I removed the backward-compatibility key reads.

Now invite-callout-storage.ts only uses:

traces:invite-callout:closed:<slug>

No fallback to old dismissed or completed keys anymore.

Validation:

Targeted ESLint passed.
bun run typecheck passed.

commit and push all changs in logical order

I’ll check the current git state first, then I’ll split your pending changes into logical commits and push.

Ran

There are 5 modified frontend files from the latest refactors. I’ll split them into two commits: useNamespaceMembers + MembersList, then invite-callout closed state changes.

Ran

First commit is in. I’m now committing the invite-callout closed key consolidation as the second logical commit, then I’ll push.

Ran

Committed and pushed in logical order.

New commits on codex/feat-invite-memebers-callout:

e37d78e1
refactor(frontend): unify namespace members loading and refresh

useNamespaceMembers now exposes loading, error, refetch
MembersList now uses the hook directly and drops local fetch effect/callback

aaa74652
refactor(frontend): collapse invite callout state to closed flag

Consolidated invite callout storage to a single closed key
Updated InviteMembersCallout and InviteManager accordingly
No legacy key compatibility retained

Push result:

Branch pushed successfully to origin/codex/feat-invite-memebers-callout
Branch is now synced (0 ahead / 0 behind).

look at our implementation and find ways we can radically simplify

I’ll audit the current org-members + invite-callout flow and give you a concrete simplification pass with highest-impact cuts first. I’m starting by reading the current hook/components end to end.

Ran

Commit is created. I’m pushing codex/feat-invite-memebers-callout to origin now.

Ran

I hit a shell parse issue on the profile layout path; I’m reopening that file with escaped path now, then I’ll give you a tight simplification map.

Ran

200 of 1868