bug:added restart dev button #2755
Conversation
|
@Surajsuthar is attempting to deploy a commit to the Onlook Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughAdds a dev-server restart flow to CodeControls: introduces an Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant CodeControls
participant SandboxSession as editorEngine.sandbox.session
participant Toast
User->>CodeControls: Click "Restart Dev Server"
CodeControls->>CodeControls: set isRestarting = true (disable button, show spinner)
CodeControls->>SandboxSession: restartDevServer()
alt success
SandboxSession-->>CodeControls: resolves
CodeControls->>Toast: toast.success("Dev server restarted")
else failure/error
SandboxSession-->>CodeControls: rejects / throws
CodeControls->>Toast: toast.error("Failed to restart dev server")
end
CodeControls->>CodeControls: finally set isRestarting = false (enable button, hide spinner)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/web/client/src/app/project/[id]/_components/right-panel/dev-tab/code-controls.tsx (3)
96-112: Prevent floating promise; keep tooltip usable when disabled
- Add void before calling the async handler in onClick to satisfy no-floating-promises rules and keep consistency with saveFile usage.
- Optional: disabled buttons often don’t trigger tooltips. Wrapping the button with a span as the TooltipTrigger keeps the tooltip accessible when disabled (same improvement applies to other disabled icon buttons in this file as a follow-up).
Apply this diff:
- <TooltipTrigger asChild> - <Button - variant="ghost" - size="icon" - disabled={isRestarting} - onClick={() => handleRestartDevServer()} - className="p-2 w-fit h-fit hover:bg-background-onlook cursor-pointer" - aria-label="Restart dev server" - > - <Icons.Reload className={cn("h-4 w-4", isRestarting && "animate-spin")}/> - </Button> - </TooltipTrigger> + <TooltipTrigger asChild> + <span> + <Button + variant="ghost" + size="icon" + disabled={isRestarting} + onClick={() => void handleRestartDevServer()} + className="p-2 w-fit h-fit hover:bg-background-onlook cursor-pointer" + aria-label="Restart dev server" + > + <Icons.Reload className={cn("h-4 w-4", isRestarting && "animate-spin")} /> + </Button> + </span> + </TooltipTrigger>
15-15: Importtoastfrom the UI wrapper for consistent themingThe UI package’s
sonner.tsxwrapper (packages/ui/src/components/sonner.tsx) re-exportstoastfrom “sonner”, ensuring it flows through our design tokens/providers. Importing directly from"sonner"bypasses our theming. Please update this file accordingly:• File:
apps/web/client/src/app/project/[id]/_components/right-panel/dev-tab/code-controls.tsx
Line 15:-import { toast } from 'sonner'; +import { toast } from '@onlook/ui/sonner';You may also want to run a repo-wide check for other direct
"sonner"imports and replace them with@onlook/ui/sonnerfor uniform styling.
41-55: Harden restart handler: rename, debounce, enrich errorsA few optional tweaks to make this handler more robust and self-documenting. None are strictly required given the current API, but they’ll improve readability and fault-tolerance:
• Rename the response variable for consistency
- const reStartResponse = await editorEngine.sandbox.session.restartDevServer(); + const restartResponse = await editorEngine.sandbox.session.restartDevServer();—matches the
Promise<boolean>return type and conventional camelCase.• Prevent concurrent restarts
if (isRestarting) return; setIsRestarting(true);—early-return stops double-clicks or repeated calls from queuing up.
• Simplify the boolean check
if (restartResponse) { toast.success('Dev server restart initiated'); } else { toast.error('Failed to restart dev server'); }Since
restartDevServer(): Promise<boolean>always resolves totrueorfalse, you can drop theundefined-handling branch.• Log and surface detailed errors
} catch (err) { console.error('restartDevServer failed', err); const message = err instanceof Error ? err.message : String(err); toast.error(`Error restarting dev server: ${message}`); } finally { setIsRestarting(false); }—captures stack traces in the console and shows the underlying message in the toast.
Overall, these are optional refactors to tighten up naming, prevent duplicate executions, and improve observability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/web/client/src/app/project/[id]/_components/right-panel/dev-tab/code-controls.tsx(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/web/client/src/app/project/[id]/_components/right-panel/dev-tab/code-controls.tsx (5)
packages/ui/src/components/sonner.tsx (1)
toast(19-19)packages/ui/src/components/tooltip.tsx (3)
Tooltip(72-72)TooltipTrigger(72-72)TooltipContent(72-72)packages/ui/src/components/button.tsx (1)
Button(57-57)packages/ui/src/components/icons/index.tsx (1)
Icons(137-3592)apps/web/template/lib/utils.ts (1)
cn(4-6)
🔇 Additional comments (2)
apps/web/client/src/app/project/[id]/_components/right-panel/dev-tab/code-controls.tsx (2)
25-25: State flag looks goodUsing a simple boolean for in-flight state is appropriate here and aligns with the rest of the component.
59-59: UI container tweaks LGTMThe alignment/gap/opacity transitions read well and preserve the existing hover affordance.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
apps/web/client/src/app/project/[id]/_components/right-panel/dev-tab/code-controls.tsx (3)
25-25: Guard against ultra-fast double clicks with a ref, not just stateState updates are async; two rapid clicks before the re-render can slip past the
isRestartingcheck. Add a ref as a synchronous, in-memory guard while keeping state for UI.-import { useState } from 'react'; +import { useRef, useState } from 'react'; @@ -const [isRestarting, setIsRestarting] = useState(false); +const [isRestarting, setIsRestarting] = useState(false); +const restartingRef = useRef(false);
96-113: Minor: pass handler directly; expose busy state; cursor when disabled
- Pass the handler directly to avoid creating a new function per render.
- Advertise busy state to assistive tech (
aria-busy).- Optionally swap the cursor when disabled for clearer affordance.
-<Button +<Button variant="ghost" size="icon" disabled={isRestarting} - onClick={() => handleRestartDevServer()} - className="p-2 w-fit h-fit hover:bg-background-onlook cursor-pointer" + onClick={handleRestartDevServer} + aria-busy={isRestarting} + className={cn( + "p-2 w-fit h-fit hover:bg-background-onlook", + isRestarting ? "cursor-not-allowed" : "cursor-pointer" + )} aria-label="Restart dev server" > <Icons.Reload className={cn("h-4 w-4", isRestarting && "animate-spin")}/> </Button>
41-56: Add a brief test plan (and optional unit test) for the restart flowGiven this is user-triggered infra control, please add a lightweight test plan to the PR description and consider a unit test that mocks the engine and verifies UI feedback.
Suggested manual test steps:
- Open a project and trigger “Restart dev server”.
- Expect: button disables, icon spins, toast says “Restart requested”.
- While spinning, attempt multiple clicks: no additional calls should be made.
- Simulate a failure (make
restartDevServerreject): expect error toast and re-enabled button.Optional Jest/RTL sketch:
// CodeControls.restart.test.tsx import { render, screen, fireEvent, waitFor } from '@testing-library/react'; import { CodeControls } from './code-controls'; jest.mock('@/components/store/editor', () => ({ useEditorEngine: () => ({ sandbox: { session: { restartDevServer: jest.fn().mockResolvedValue(true) } }, ide: { activeFile: { isDirty: false }, files: [], saveActiveFile: jest.fn() }, }), })); test('invokes restart once and disables while in-flight', async () => { render(<CodeControls />); const btn = await screen.findByRole('button', { name: /restart dev server/i }); fireEvent.click(btn); // second click should be ignored fireEvent.click(btn); await waitFor(() => expect(btn).toHaveAttribute('aria-busy', 'true')); // assert toast and re-enable happen await waitFor(() => expect(btn).toHaveAttribute('aria-busy', 'false')); });If you’d like, I can open a follow-up PR to add this test.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/web/client/src/app/project/[id]/_components/right-panel/dev-tab/code-controls.tsx(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/web/client/src/app/project/[id]/_components/right-panel/dev-tab/code-controls.tsx (2)
packages/ui/src/components/sonner.tsx (1)
toast(19-19)packages/ui/src/components/icons/index.tsx (1)
Icons(137-3592)
🔇 Additional comments (2)
apps/web/client/src/app/project/[id]/_components/right-panel/dev-tab/code-controls.tsx (2)
15-15: LGTM: toast importImport looks correct and consistent with the UI package usage.
60-60: LGTM: container spacing and hover behaviorThe added alignment and gap styles integrate well with adjacent controls.
| const handleRestartDevServer = async () => { | ||
| try { | ||
| if (isRestarting) return; | ||
| setIsRestarting(true); | ||
| const restartResponse = await editorEngine.sandbox.session.restartDevServer(); | ||
| if (restartResponse) { | ||
| toast.success('Dev server restarting'); | ||
| } else { | ||
| toast.error('Failed to restart dev server'); | ||
| } | ||
| } catch (err) { | ||
| toast.error('Error restarting dev server'); | ||
| } finally { | ||
| setIsRestarting(false); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden restart handler: atomic reentrancy guard, null-check engine path, better error detail
- Use the ref guard to prevent concurrent calls.
- Null-check
editorEngine?.sandbox?.session?.restartDevServerto avoid runtime errors when the session is unavailable. - Log error details for debugging and surface a concise user-facing toast.
-const handleRestartDevServer = async () => {
- try {
- if (isRestarting) return;
- setIsRestarting(true);
- const restartResponse = await editorEngine.sandbox.session.restartDevServer();
- if (restartResponse) {
- toast.success('Dev server restarting');
- } else {
- toast.error('Failed to restart dev server');
- }
- } catch (err) {
- toast.error('Error restarting dev server');
- } finally {
- setIsRestarting(false);
- }
-}
+const handleRestartDevServer = async () => {
+ if (restartingRef.current) return; // fast re-entry guard
+ restartingRef.current = true;
+ setIsRestarting(true);
+ try {
+ const restartFn = editorEngine?.sandbox?.session?.restartDevServer;
+ if (typeof restartFn !== 'function') {
+ toast.error('Restart unavailable in this context');
+ return;
+ }
+ const ok = await restartFn();
+ if (ok) {
+ toast.success('Dev server restart requested');
+ } else {
+ toast.error('Failed to request dev server restart');
+ }
+ } catch (err) {
+ // Keep the toast concise, but log details for diagnostics
+ // eslint-disable-next-line no-console
+ console.error('Error restarting dev server', err);
+ const msg = err instanceof Error ? err.message : 'Unknown error';
+ toast.error(`Error restarting dev server: ${msg}`);
+ } finally {
+ setIsRestarting(false);
+ restartingRef.current = false;
+ }
+};Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
apps/web/client/src/app/project/[id]/_components/right-panel/dev-tab/code-controls.tsx
around lines 41 to 56, the restart handler lacks an atomic reentrancy guard,
doesn't null-check the nested engine path before invoking restartDevServer, and
swallows error details; add a useRef boolean (e.g., restartingRef) to serve as
the atomic guard (check and set it immediately at function start and clear in
finally), verify editorEngine?.sandbox?.session?.restartDevServer is a function
before awaiting it (return early with a user toast if not available), and catch
errors by logging detailed error info (console.error or app logger) while
showing a concise toast message to the user; ensure the ref and setIsRestarting
are cleared in finally so state is consistent.
|
Oh this is nice. Could we move the button to next to the Dev Server CLI on the bottom bar perhaps? @Surajsuthar |
|
@Kitenite sure if you have any specific design lmk |
Description
Added button to restart dev server
Related Issues
"bug #2749"
Type of Change
Testing
Screenshots (if applicable)
Screen.Recording.2025-08-25.at.2.38.38.PM.mov
Additional Notes
Important
Adds a "Restart dev server" button to
CodeControlswith loading state and user feedback usingtoast.CodeControlscomponent incode-controls.tsx.isRestartingstate.toastbased on restart outcome.isRestartingstate to manage button's disabled state and loading animation.This description was created by
for f7606a6. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
UX/UI