Refactor: Decompose monolithic MainPlaygroundPage into modular components#65
Open
rishivejani15 wants to merge 4 commits into
Open
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughSplits the monolithic playground page into a shared PlaygroundContext, a modal Zustand store, a usePlaygroundActions hook (save/download/CRUD), and modular UI components (Layout, Header, Sidebar, MainArea, ModalOrchestrator). The page now provides context and renders composed components with hydration/useEffect guards. ChangesPlayground Refactoring
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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: 9
🧹 Nitpick comments (3)
modules/playground/hooks/usePlaygroundShortcuts.ts (1)
44-47: ⚡ Quick winInconsistent store access pattern.
Line 46 directly calls
useAI.getState().toggleChat()instead of using the hook selector pattern (like lines 12-14). While this works in Zustand, it's inconsistent with the rest of the codebase and harder to understand.♻️ Refactor for consistency
export function usePlaygroundShortcuts() { const { handleSave, handleSaveAll } = usePlaygroundActions(); const sidebar = useSidebar(); const toggleCommandPalette = useModalStore(state => state.toggleCommandPalette); const togglePreview = useModalStore(state => state.togglePreview); + const toggleAIChat = useAI(state => state.toggleChat); const { activeFileId, closeFile } = useFileExplorer(); useEffect(() => { const handleKeyDown = (e: KeyboardEvent) => { // ... other shortcuts ... // Ctrl+Shift+A — Toggle AI Chat if (e.ctrlKey && e.shiftKey && e.key === "A") { e.preventDefault(); - useAI.getState().toggleChat(); + toggleAIChat(); } // ... }; window.addEventListener("keydown", handleKeyDown); return () => window.removeEventListener("keydown", handleKeyDown); - }, [handleSave, handleSaveAll, sidebar, activeFileId, closeFile, toggleCommandPalette, togglePreview]); + }, [handleSave, handleSaveAll, sidebar, activeFileId, closeFile, toggleCommandPalette, togglePreview, toggleAIChat]); }🤖 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 `@modules/playground/hooks/usePlaygroundShortcuts.ts` around lines 44 - 47, The shortcut handler uses useAI.getState().toggleChat() directly which is inconsistent with the selector-hook pattern used elsewhere; change this to read the toggle function via the hook selector (e.g., const toggleChat = useAI(state => state.toggleChat)) and call toggleChat() inside the Ctrl+Shift+A branch so the code matches the selector usage found around lines using useAI(state => ...).modules/playground/components/playground-header.tsx (1)
109-109: 💤 Low valueUnnecessary arrow function wrapper.
togglePreviewis already a zero-argument function, so the arrow wrapper() => togglePreview()can be simplified to justtogglePreview.- onClick={() => togglePreview()} + onClick={togglePreview}🤖 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 `@modules/playground/components/playground-header.tsx` at line 109, Replace the unnecessary arrow wrapper used in the JSX onClick prop with the direct function reference: instead of using "() => togglePreview()", pass togglePreview directly to onClick (locate the onClick assignment in the PlaygroundHeader component where togglePreview is referenced) so the zero-argument handler is invoked correctly without creating an extra closure.modules/playground/components/playground-main-area.tsx (1)
78-78: ⚡ Quick winNon-null assertion on
templateDatabypasses type safety.The context types
templateDataasTemplateFolder | null, but this line usestemplateData!to assert it's non-null. While the parent page checkstemplateDataexists before rendering, relying on assertions reduces compile-time safety if the render flow changes.🔧 Safer approach
Since
openFiles.length > 0already guards this code path, and you're inside the context wheretemplateDatawas checked at the page level, consider:
- Add an early return at the component top if
templateDatais null, or- Update the context type to guarantee
templateDatais non-null when the provider renders (by not mounting children until it's loaded).Option 2 is cleaner and would eliminate the need for assertions throughout the codebase.
🤖 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 `@modules/playground/components/playground-main-area.tsx` at line 78, The code uses a non-null assertion on templateData when rendering <Breadcrumbs activeFile={activeFile} templateData={templateData!} />, which bypasses type safety; fix it by ensuring templateData is statically non-null before use—either add an early return at the top of the PlaygroundMainArea component when templateData is null (e.g., if (!templateData || openFiles.length === 0) return null) or update the provider/context so children only mount when templateData is populated (thus changing the context type to TemplateFolder without null), then remove the templateData! assertion from the Breadcrumbs invocation.
🤖 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 `@app/playground/`[id]/page.tsx:
- Around line 35-40: The effect currently mixes initialization and template
syncing and uses openFiles.length in the dependency array, causing missed
initialization when openFiles is pre-populated and unnecessary reruns; split
into two effects: one useEffect that only calls setPlaygroundId(id) with
dependency [id] (and setPlaygroundId) and a second effect that initializes
template data by calling setTemplateData(templateData) when templateData is
present but only on mount or when templateData itself changes (do NOT depend on
openFiles.length) so setTemplateData is not prevented when openFiles is already
populated; update references to setPlaygroundId, setTemplateData, templateData,
and openFiles accordingly.
- Around line 32-33: Remove the // `@ts-ignore` and fix the mismatch between the
hook return and the destructuring: update the useWebContainer hook to return the
correctly spelled "destroy" (instead of "destory") and then include that
property in the destructuring in page.tsx (or include it as an unused name like
_destroy if you don't use it). Alternatively, if the cleanup callback is truly
unnecessary, remove the property from useWebContainer's return type; in either
case ensure the destructured names in page.tsx exactly match the hook's exported
properties (useWebContainer and destroy/destory) and remove the ts-ignore.
In `@modules/playground/components/playground-context.tsx`:
- Around line 4-14: The PlaygroundContextType interface currently uses any for
playgroundData, instance, and writeFileSync; replace those anys with explicit
types (e.g., define/import a PlaygroundData type for playgroundData, a
PlaygroundInstance or ContainerInstance type for instance, and a WriteFileSync
function signature or FileSystemAdapter type for writeFileSync) and update the
interface to use those symbols (PlaygroundContextType, playgroundData, instance,
writeFileSync); add the necessary imports or local type declarations (and make
writeFileSync a typed function like (path: string, content: string | Buffer,
options?: any) => void or a Promise-returning variant if async) so the file
gains strict typing and IDE help.
In `@modules/playground/components/playground-main-area.tsx`:
- Around line 93-107: The code passes serverUrl! into WebContainerPreview even
though PlaygroundContextType declares serverUrl: string | null and
containerStatus can make it null; either prevent rendering the preview when
serverUrl is null or make WebContainerPreview accept nullable serverUrl. Fix by
adding a runtime guard to the isPreviewVisible branch (e.g., only render the
ResizablePanel/WebContainerPreview when isPreviewVisible && serverUrl != null)
or change WebContainerPreview's prop type to serverUrl: string | null and update
its implementation to handle null/undefined safely; update references to
serverUrl, WebContainerPreview, and isPreviewVisible accordingly.
In `@modules/playground/components/playground-sidebar.tsx`:
- Line 75: The code uses non-null assertions on templateData (templateData!) in
multiple JSX render paths which can crash if the context is briefly
uninitialized; replace those assertions by guarding access to templateData:
either return a safe fallback/loader when templateData is falsy or conditionally
render the sections (e.g. templateData && <...> or use optional chaining like
templateData?.prop) so the sidebar doesn't attempt to read from undefined;
update all occurrences that currently use templateData! (the three render paths
in this component) to use these guards.
In `@modules/playground/hooks/usePlaygroundActions.ts`:
- Around line 38-39: handleSave and handleSaveAll are using a closure-captured
openFiles snapshot (e.g., the openFiles.find in usePlaygroundActions) which can
be stale and let concurrent saves clobber hasUnsavedChanges clears; update the
handlers to read the latest openFiles at call-time instead of relying on the
closed-over variable—for example, use a stable ref or a state getter
(openFilesRef.current or getOpenFiles()) or accept the current files as a
parameter, then locate the target file (fileToSave) from that fresh value and
update hasUnsavedChanges atomically; apply the same change to the other affected
handlers around the blocks identified (the handleSave block and the sections at
lines referenced 83-93 and 105-113) so all save/save-all code paths use the
up-to-date openFiles snapshot.
- Around line 57-69: handleSave's updateFileContent currently matches files by
filename and fileExtension which can overwrite duplicates in different folders;
change updateFileContent to carry and compare the full path during recursion
instead of only filename+fileExtension. Specifically, modify the
updateFileContent function to accept a currentPath parameter (build it when
recursing through folder items) and compare that constructed path to
fileToSave.path (or fileToSave.fullPath) when deciding to replace content;
update references to updatedTemplateData.items so the recursive calls supply the
path context. Ensure the matching uses the unique full/path string rather than
filename and fileExtension alone.
- Around line 153-156: wrappedHandleAddFile currently force-unwraps
writeFileSync and may throw if the filesystem/container isn’t ready; update the
useCallback wrapper for wrappedHandleAddFile to guard that writeFileSync is
defined before calling handleAddFile (e.g., if (!writeFileSync) return or
queue/fail gracefully), and pass instance and saveTemplateData only when safe;
locate the wrapper in usePlaygroundActions.ts (function wrappedHandleAddFile
referencing handleAddFile, writeFileSync, instance, saveTemplateData) and add
the null-check and appropriate early return or fallback behavior.
In `@modules/playground/hooks/usePlaygroundShortcuts.ts`:
- Around line 49-54: In usePlaygroundShortcuts.ts, the key handler calls
e.preventDefault() for Ctrl+W before checking activeFileId which blocks normal
browser behavior; change the logic so e.preventDefault() is only invoked when
there is an active file to close (i.e., check activeFileId first, and if present
call e.preventDefault() then closeFile(activeFileId)), leaving browser default
behavior intact when no file is active.
---
Nitpick comments:
In `@modules/playground/components/playground-header.tsx`:
- Line 109: Replace the unnecessary arrow wrapper used in the JSX onClick prop
with the direct function reference: instead of using "() => togglePreview()",
pass togglePreview directly to onClick (locate the onClick assignment in the
PlaygroundHeader component where togglePreview is referenced) so the
zero-argument handler is invoked correctly without creating an extra closure.
In `@modules/playground/components/playground-main-area.tsx`:
- Line 78: The code uses a non-null assertion on templateData when rendering
<Breadcrumbs activeFile={activeFile} templateData={templateData!} />, which
bypasses type safety; fix it by ensuring templateData is statically non-null
before use—either add an early return at the top of the PlaygroundMainArea
component when templateData is null (e.g., if (!templateData || openFiles.length
=== 0) return null) or update the provider/context so children only mount when
templateData is populated (thus changing the context type to TemplateFolder
without null), then remove the templateData! assertion from the Breadcrumbs
invocation.
In `@modules/playground/hooks/usePlaygroundShortcuts.ts`:
- Around line 44-47: The shortcut handler uses useAI.getState().toggleChat()
directly which is inconsistent with the selector-hook pattern used elsewhere;
change this to read the toggle function via the hook selector (e.g., const
toggleChat = useAI(state => state.toggleChat)) and call toggleChat() inside the
Ctrl+Shift+A branch so the code matches the selector usage found around lines
using useAI(state => ...).
🪄 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: 588fb100-bc20-4994-91d0-73f33b159d1a
📒 Files selected for processing (10)
app/playground/[id]/page.tsxmodules/playground/components/playground-context.tsxmodules/playground/components/playground-header.tsxmodules/playground/components/playground-layout.tsxmodules/playground/components/playground-main-area.tsxmodules/playground/components/playground-modal-orchestrator.tsxmodules/playground/components/playground-sidebar.tsxmodules/playground/hooks/useModalStore.tsmodules/playground/hooks/usePlaygroundActions.tsmodules/playground/hooks/usePlaygroundShortcuts.ts
Owner
piyushdotcomm
left a comment
piyushdotcomm
left a comment
There was a problem hiding this comment.
Nice work on the decomposition, this makes the playground much more maintainable.
A few things to fix:
- In usePlaygroundShortcuts, move e.preventDefault() inside the activeFileId check for Ctrl+W. Currently, it prevents closing the browser tab even when no file is open.
- Let's keep this focused on the refactor—move the new shortcut system to a separate PR so we can audit the keybindings independently.
Clean those up and I'm happy to merge.
Author
|
please check |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/playground/[id]/page.tsx (1)
3-22:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winDuplicate and incomplete React imports will cause build errors.
React is imported twice (lines 3 and 16-22), which will cause a syntax error. Additionally, several hooks (
useCallback,useRef,useState) appear unused in this file after the refactor.🐛 Proposed fix to consolidate imports
-import React, { useEffect } from "react"; +import React, { Suspense, useEffect } from "react"; import { useParams } from "next/navigation"; import { AlertCircle, FolderOpen } from "lucide-react"; import { Button } from "@/components/ui/button"; import PlaygroundSkeleton from "@/modules/playground/components/loader"; import { usePlayground } from "@/modules/playground/hooks/usePlayground"; import { useFileExplorer } from "@/modules/playground/hooks/useFileExplorer"; import { findFilePath } from "@/modules/playground/lib"; import { TemplateFile, TemplateFolder, } from "@/modules/playground/lib/path-to-json"; -import React, { - Suspense, - useCallback, - useEffect, - useRef, - useState, -} from "react"; import { toast } from "sonner";🤖 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 `@app/playground/`[id]/page.tsx around lines 3 - 22, There are duplicate React imports and unused hook imports causing build errors; remove the extra default and named duplicate (the second block that redeclares "import React, { Suspense, useCallback, useRef, useState, useEffect } from 'react'") and consolidate into a single import that includes only the used symbols (e.g., keep "import React, { useEffect } from 'react'" or add Suspense if used); also remove any unused hooks (useCallback, useRef, useState) from the import list or reintroduce their usages where intended so the file's imports (e.g., usePlayground, useFileExplorer, findFilePath, TemplateFile, TemplateFolder) compile cleanly.
🧹 Nitpick comments (1)
app/playground/[id]/page.tsx (1)
11-23: ⚡ Quick winRemove unused imports.
After the refactor,
findFilePath,TemplateFile,TemplateFolder, andtoastappear unused in this file. Clean them up to reduce noise.♻️ Proposed cleanup
import { usePlayground } from "@/modules/playground/hooks/usePlayground"; import { useFileExplorer } from "@/modules/playground/hooks/useFileExplorer"; -import { findFilePath } from "@/modules/playground/lib"; -import { - TemplateFile, - TemplateFolder, -} from "@/modules/playground/lib/path-to-json"; -import { toast } from "sonner";🤖 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 `@app/playground/`[id]/page.tsx around lines 11 - 23, The imports findFilePath, TemplateFile, TemplateFolder, and toast are no longer used in this module; remove them from the import list to eliminate dead code and linter warnings. Edit the import statements that reference findFilePath and path-to-json types to drop those symbols, and remove the toast import from "sonner"; keep only the React/Suspense/useCallback/useEffect/useRef/useState imports that are actually used in page.tsx (and adjust comma/brace formatting if needed).
🤖 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 `@app/playground/`[id]/page.tsx:
- Around line 25-29: The file references PlaygroundContext.Provider and calls
useWebContainer but neither symbol is imported; add named imports for
PlaygroundContext and useWebContainer at the top of the file (matching their
actual module exports) so the component can call useWebContainer(...) and wrap
children with <PlaygroundContext.Provider ...> without runtime ReferenceErrors;
ensure the import names exactly match the exported identifiers used in the file
(PlaygroundContext, useWebContainer).
---
Outside diff comments:
In `@app/playground/`[id]/page.tsx:
- Around line 3-22: There are duplicate React imports and unused hook imports
causing build errors; remove the extra default and named duplicate (the second
block that redeclares "import React, { Suspense, useCallback, useRef, useState,
useEffect } from 'react'") and consolidate into a single import that includes
only the used symbols (e.g., keep "import React, { useEffect } from 'react'" or
add Suspense if used); also remove any unused hooks (useCallback, useRef,
useState) from the import list or reintroduce their usages where intended so the
file's imports (e.g., usePlayground, useFileExplorer, findFilePath,
TemplateFile, TemplateFolder) compile cleanly.
---
Nitpick comments:
In `@app/playground/`[id]/page.tsx:
- Around line 11-23: The imports findFilePath, TemplateFile, TemplateFolder, and
toast are no longer used in this module; remove them from the import list to
eliminate dead code and linter warnings. Edit the import statements that
reference findFilePath and path-to-json types to drop those symbols, and remove
the toast import from "sonner"; keep only the
React/Suspense/useCallback/useEffect/useRef/useState imports that are actually
used in page.tsx (and adjust comma/brace formatting if needed).
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 5cea1d67-64ee-4d00-911f-d1d1d874a1f3
📒 Files selected for processing (10)
app/playground/[id]/page.tsxmodules/playground/components/playground-context.tsxmodules/playground/components/playground-header.tsxmodules/playground/components/playground-layout.tsxmodules/playground/components/playground-main-area.tsxmodules/playground/components/playground-modal-orchestrator.tsxmodules/playground/components/playground-sidebar.tsxmodules/playground/components/welcome-screen.tsxmodules/playground/hooks/useModalStore.tsmodules/playground/hooks/usePlaygroundActions.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- modules/playground/components/playground-sidebar.tsx
- modules/playground/components/playground-context.tsx
- modules/playground/components/playground-layout.tsx
- modules/playground/components/playground-modal-orchestrator.tsx
- modules/playground/components/playground-header.tsx
- modules/playground/components/playground-main-area.tsx
- modules/playground/hooks/useModalStore.ts
- modules/playground/hooks/usePlaygroundActions.ts
Owner
|
resolve the conflicts |
piyushdotcomm
requested changes
May 14, 2026
Owner
piyushdotcomm
left a comment
piyushdotcomm
left a comment
There was a problem hiding this comment.
checks are still failing and merge conflict is also there
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
2 participants
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.
Overview
This PR addresses the technical debt in
app/playground/[id]/page.tsxby decomposing it from a massive ~600-line monolithic "God Component" into a clean, thin compositor.Previously,
MainPlaygroundPageacted as a massive orchestrator that bridged state from multiple hooks and prop-drilled handlers 3-4 levels deep. This refactor breaks the UI and logic into focused, maintainable pieces without altering any of the underlying playground functionality.Key Changes
useModalStore) to manage all application-wide toggles and dialog states (Preview, AI Settings, Command Palette, Deploy Dialog), removinguseStateclutter from the main page.PlaygroundContextto provide essential base data (templateData,instance,writeFileSync) cleanly down the component tree.usePlaygroundActionshook.PlaygroundLayout: Manages top-level wrappers and binds global keyboard shortcuts (via the newusePlaygroundShortcutshook).PlaygroundMainArea: Encapsulates the header, tab bar, editor, and preview pane.PlaygroundModalOrchestrator: A dedicated component for rendering all modals, driven entirely byuseModalStore.PlaygroundSidebar: Updated to consume hooks/context directly rather than relying on massive prop chains.Result
app/playground/[id]/page.tsxhas been successfully reduced to ~85 lines. It now strictly handles data fetching, loading/error states, and context provision, dramatically improving readability, testability, and reducing the likelihood of future merge conflicts.Summary by CodeRabbit
New Features
Refactor
Bug Fixes