chore(studio): react-doctor follow-up — state reducers + component decomposition#147
Merged
iipanda merged 3 commits intoMay 18, 2026
Merged
Conversation
- api-key-create-dialog: 6 form useStates collapsed into a single reducer (step, label, selectedScopes, expiresAt, createdResult, copied, submitError). Reset effect now dispatches `reset` instead of cascading individual setters; todayMinDate recompute moved into the same on-open effect. - users-page: extracted InviteUserDialog (with its own reducer), EditRoleDialog, PendingInvitesList, and UsersTable. UsersPage orchestrator dropped from 576 to ~160 lines. Remaining (deferred to follow-up worktree): - environments-page giant-component split - content-document-page section extractions - content/[type]/page, trash-page, layout, inline-ai-bubble, assistant-context
- trash-page: TrashFilterBar, TrashEmptyMatch, TrashTable, TrashPagination extracted. TrashPage body drops from ~390 lines to ~190. - content/[type]/page: ContentTypeDocumentsTable + ContentTypePaginationBar extracted. ContentTypePage shrinks but is still above threshold due to query/mutation wiring; a deeper split is deferred. - users-page formatting pass after prior reducer + decomposition commit. Drops `no-giant-component` count by 1 (trash-page no longer flagged).
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughThis PR extracts table/pagination/filter/dialog UI into internal components across ContentTypePage, TrashPage, and UsersPage, and consolidates ApiKeyCreateDialog form state with a reducer. No exported/public signatures changed. ChangesAdmin UI Component Extraction and State Consolidation
Possibly related PRs:
🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
packages/studio/src/lib/runtime-ui/components/api-key-create-dialog.tsx (1)
118-144: ⚡ Quick winReturn a fresh state object in the reset case.
The
"reset"case returnsinitialFormStatedirectly, which means all reset operations share the sameSetinstance forselectedScopes. While the current reducer implementation never mutates Sets directly (always creating new instances), this pattern is fragile—if future code accidentally mutatesstate.selectedScopesdirectly, it would corrupt the shared initial state.♻️ Proposed fix to return a fresh state on reset
Option 1 (inline):
function formReducer(state: FormState, action: FormAction): FormState { switch (action.type) { case "reset": - return initialFormState; + return { ...initialFormState, selectedScopes: new Set() };Option 2 (factory function):
-const initialFormState: FormState = { +const getInitialFormState = (): FormState => ({ step: "form", label: "", selectedScopes: new Set(), expiresAt: "", createdResult: null, copied: false, submitError: null, -}; +}); function formReducer(state: FormState, action: FormAction): FormState { switch (action.type) { case "reset": - return initialFormState; + return getInitialFormState();Then update the useReducer call:
-const [form, dispatch] = useReducer(formReducer, initialFormState); +const [form, dispatch] = useReducer(formReducer, getInitialFormState());🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/studio/src/lib/runtime-ui/components/api-key-create-dialog.tsx` around lines 118 - 144, The reset branch in formReducer currently returns the shared initialFormState object, risking shared mutable state for selectedScopes; change the "reset" case to return a fresh state object (not initialFormState) by copying primitive fields and creating a new Set for selectedScopes (e.g., new Set(initialFormState.selectedScopes)) so formReducer always returns independent state on "reset"; update any useReducer initialization only if you adopt an initial state factory but ensure selectedScopes is newly constructed when resetting.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/studio/src/lib/runtime-ui/app/admin/content/`[type]/page.tsx:
- Around line 431-433: The pager currently always renders pages 1..5 and moves
offset without bounds; update the page-range calculation (the Array.from mapping
that builds page links) to compute a clamped window based on currentPage and
totalPages (e.g. compute start = Math.max(1, Math.min(currentPage - 2,
totalPages - 4)) and end = Math.min(totalPages, start + 4)) so the rendered
pages shift when currentPage > 5, and replace the direct onPageChange(offset +
PAGE_SIZE) calls with a bounded calculation that clamps the new offset to [0,
(totalPages - 1) * PAGE_SIZE] (using Math.max/Math.min) before calling
onPageChange; update both occurrences (the Array.from pagination block and the
handler at the other occurrence) and keep using the existing symbols
currentPage, totalPages, PAGE_SIZE, offset, and onPageChange.
- Around line 335-339: The TableRow currently only supports mouse clicks; make
it keyboard-accessible by adding tabbable and ARIA affordances: give the
TableRow (the element rendering the row with key={doc.documentId}) a
tabIndex={0}, role="button" (or role="link" if more appropriate) and an
onKeyDown handler that calls onRowClick(doc.documentId) when Enter or Space is
pressed; also ensure any existing onClick still calls onRowClick and add an
appropriate aria-label or aria-labelledby so screen readers announce the row
action.
In `@packages/studio/src/lib/runtime-ui/app/admin/trash-page.tsx`:
- Around line 309-311: Pagination currently renders a fixed window of pages 1–5
using Array.from({ length: Math.min(5, totalPages) }) which ignores currentPage;
change the page window calculation to center on currentPage (e.g., compute start
= Math.max(1, Math.min(currentPage - 2, totalPages - 4)) and end =
Math.min(totalPages, start + 4)) and render pages from start..end so the visible
page links move with currentPage; also update the page-change handler(s) that
compute next/previous pages (the onClick/onChange that sets currentPage) to
clamp the new page with Math.min/Math.max against 1 and totalPages so
advancing/offsets never exceed bounds.
In `@packages/studio/src/lib/runtime-ui/app/admin/users-page.tsx`:
- Around line 364-404: handleSave currently calls updateGrants without error
handling; wrap the async updateGrants call in a try/catch inside handleSave,
introduce a component state variable (e.g., error and setError) to store any
error message, on success proceed to call onSaved(target.userName) and
onOpenChange(false) but on failure setError with a user-friendly message (or
error.message) and do not close the dialog, and update the dialog JSX to render
{error && <p className="text-sm text-destructive">{error}</p>} similar to
InviteUserDialog so users see the failure.
---
Nitpick comments:
In `@packages/studio/src/lib/runtime-ui/components/api-key-create-dialog.tsx`:
- Around line 118-144: The reset branch in formReducer currently returns the
shared initialFormState object, risking shared mutable state for selectedScopes;
change the "reset" case to return a fresh state object (not initialFormState) by
copying primitive fields and creating a new Set for selectedScopes (e.g., new
Set(initialFormState.selectedScopes)) so formReducer always returns independent
state on "reset"; update any useReducer initialization only if you adopt an
initial state factory but ensure selectedScopes is newly constructed when
resetting.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 73a1a4ca-52ff-48c8-a820-1cab43e9c272
📒 Files selected for processing (4)
packages/studio/src/lib/runtime-ui/app/admin/content/[type]/page.tsxpackages/studio/src/lib/runtime-ui/app/admin/trash-page.tsxpackages/studio/src/lib/runtime-ui/app/admin/users-page.tsxpackages/studio/src/lib/runtime-ui/components/api-key-create-dialog.tsx
- ContentTypePaginationBar + TrashPagination: page window is now centered on
currentPage (clamped to [1, totalPages - 4..0]) instead of always 1..5, and
every navigation handler clamps the new offset to [0, (totalPages-1)*PAGE_SIZE]
so prev/next can't escape the bounds.
- ContentTypeDocumentsTable rows are now keyboard-accessible: tabIndex=0,
role="button", aria-label="Open document {title}", and an onKeyDown that
invokes the same onRowClick on Enter or Space.
- EditRoleDialog handleSave wraps updateGrants in try/catch, stores an
error string in component state, renders it below the form fields, and
leaves the dialog open on failure so the user sees what went wrong.
- formReducer "reset" no longer returns the shared initialFormState (whose
`selectedScopes` Set could be mutated through the module-level reference).
Switched to a createInitialFormState() factory and call it from the reset
branch to hand back a fresh Set each time.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #146. Addresses the deferred work from the previous react-doctor cleanup pass:
prefer-useReducer,no-cascading-set-state(where naturally combinable with reducers), and the smallerno-giant-componentextractions. All edits land inruntime-ui/**paths declared asunpublishedSourcesin.changeset-gate.json, so no changeset is required.Changes
useReducer migrations
api-key-create-dialog.tsx: 6 formuseStatecalls + the cascading reset effect collapsed into one discriminated-union reducer (label-change/scope-toggle/expires-at-change/submit-*/copy-set/reset). The on-open effect now both recomputestodayMinDateand dispatchesreseton close.users-page.tsxinvite form: the 3-field invite state plus its error string moved into aninviteFormReducerinside the newInviteUserDialog. The parent no longer owns invite form state.Component decomposition
users-page.tsx(was 700 lines / 576-line body) → page orchestrator (~165 lines) +InviteUserDialog,EditRoleDialog,PendingInvitesList,UsersTable. Page is no longer flagged as a giant component.trash-page.tsx(was ~390-line body) →TrashFilterBar,TrashTable,TrashEmptyMatch,TrashPaginationextracted. Page is no longer flagged.content/[type]/page.tsx:ContentTypeDocumentsTable,ContentTypePaginationBarextracted. The page is still over the threshold because of query/mutation wiring; deeper decomposition deferred.Doctor-score delta
@mdcms/studiono-giant-componentflagged sitesThe numeric score is unchanged because the remaining giant components (content-document-page ×2, environments-page, assistant-context provider, layout, inline-ai-bubble) and the documented false-positives still account for the warning population. A separate PR can tackle them.
Deferred (out of scope for this PR)
content-document-page.tsxContentDocumentPageView (576-line body) and ContentDocumentPage (1142-line body) — the document editor is the most state-dense surface in the studio; a careful split needs its own design pass.environments-page.tsx(563-line body, 17+ useStates) — the same; a focused state-machine refactor would be cleaner than mechanical extractions.assistant-context.tsxAssistantProvider (615-line body) — provider with deeply interconnected chat dispatch state; pulling a hook out is fine but doesn't move the lint number cleanly.inline-ai-bubble.tsx(manageable but the twono-effect-chainsites are legitimate debounce + async-state lifecycles — see commit on the original PR).layout.tsxAdminLayoutInner (235-line body) — auth gating + query setup is interleaved; a clean split requires moving the early-returns through a new gate component.These are intentionally out of scope and can ship as separate focused PRs.
Test plan
bun run check(build + typecheck across 6 projects) — green.bun test --cwd packages/studio— 603/603 pass.bun run format:check— no studio-touching files dirty.bun run ci:requiredin CI.Summary by CodeRabbit