Feat: Download Projects, Deploy to Vercel + Confetti Celebrations#5
Feat: Download Projects, Deploy to Vercel + Confetti Celebrations#5
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThis pull request introduces a project download feature with ZIP archive generation, adds a Deploy to Vercel button with GitHub URL integration, and migrates API calls from environment-based URLs to proxy endpoints. Client-side features include confetti animation and file polling, while server-side enhancements add file size gating to prevent large file transmission. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45–75 minutes Areas requiring extra attention:
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/client/src/store/project.ts (1)
105-116: Handle non-2xx responses infetchFiles
res.json()on error responses can throw and you lose status/context.- const res = await fetch(`/api/proxy/projects/${projectId}/files`); - const data = await res.json(); + const res = await fetch(`/api/proxy/projects/${projectId}/files`); + if (!res.ok) throw new Error(`fetchFiles failed: ${res.status} ${res.statusText}`); + const data = await res.json();apps/server/src/services/workspace.ts (1)
79-113: Fix path separator consistency (Windows breaks client tree parsing)
Client-side tree building typically assumes/. On Windows,path.joinwill produce\, causing incorrect nesting/selection.- const relPath = path.join(relativePath, entry.name); + const relPath = path.posix.join(relativePath, entry.name);Also consider guarding
fs.statper-file so one bad entry doesn’t fail the whole listing.apps/client/src/components/agent/PreviewPanel.tsx (1)
102-137: Toolbar structure is broken with duplicate elements.The new elements (divider, Deploy button, refresh, and external link) are placed outside the flex container that closes at line 114. Additionally, there are duplicate
RefreshCwandExternalLinkbuttons—one set inside the container (lines 108-113) and another outside (lines 132-137).This will cause:
- Layout issues—new elements won't be aligned within the toolbar pill
- Duplicate buttons rendered to the user
Move all toolbar elements inside the flex container and remove duplicates:
<div className="flex items-center gap-2 px-3 py-1.5 bg-[#0A0A0B]/80 backdrop-blur-md rounded-full border border-white/10 shadow-xl shadow-indigo-500/10"> <span className="text-[10px] text-zinc-400 font-mono max-w-[150px] truncate"> /preview/{projectId} </span> <div className="h-3 w-px bg-white/10" /> + {githubUrl && ( + <> + <a + href={`https://vercel.com/new/import?repository-url=${encodeURIComponent(githubUrl)}`} + target="_blank" + rel="noopener noreferrer" + className="flex items-center gap-1 text-xs font-medium text-white bg-black hover:bg-neutral-900 px-2 py-0.5 rounded transition-colors" + > + <Rocket className="w-3 h-3" /> + Deploy + </a> + <div className="h-3 w-px bg-white/10" /> + </> + )} <button onClick={handleRefresh} className="text-zinc-400 hover:text-white transition-colors"> <RefreshCw className={cn("w-3 h-3", status === 'loading' && "animate-spin")} /> </button> <a href={previewUrl} target="_blank" rel="noopener noreferrer" className="text-zinc-400 hover:text-white transition-colors"> <ExternalLink className="w-3 h-3" /> </a> </div> - <div className="h-3 w-px bg-white/10" /> - - {githubUrl && ( - <> - <a - href={`https://vercel.com/new/import?repository-url=${encodeURIComponent(githubUrl)}`} - target="_blank" - rel="noopener noreferrer" - className="flex items-center gap-1 text-xs font-medium text-white bg-black hover:bg-neutral-900 px-2 py-0.5 rounded transition-colors" - > - <Rocket className="w-3 h-3" /> - Deploy - </a> - <div className="h-3 w-px bg-white/10" /> - </> - )} - - <button onClick={handleRefresh} className="text-zinc-400 hover:text-white transition-colors"> - <RefreshCw className={cn("w-3 h-3", status === 'loading' && "animate-spin")} /> - </button> - <a href={previewUrl} target="_blank" rel="noopener noreferrer" className="hover:text-blue-400 transition-colors"> - <ExternalLink className="w-3 h-3" /> - </a>
🧹 Nitpick comments (2)
apps/client/src/store/project.ts (1)
46-46: Avoidio(""); prefer explicit same-origin or fail-fast in prod
Passing an empty string can mask misconfiguration and produce environment-dependent socket targets.-const API_URL = process.env.NEXT_PUBLIC_API_URL || ""; +const API_URL = process.env.NEXT_PUBLIC_API_URL;- const socket = io(API_URL); + const socket = io(API_URL ?? undefined); // lets socket.io-client pick current origin + // or: io(API_URL ?? window.location.origin)apps/client/src/components/agent/FileSystemPanel.tsx (1)
20-30: Polling: avoid overlappingfetchFilescalls under slow networks
Current interval can trigger concurrent requests; consider a simple in-flight guard.- const interval = setInterval(() => { - fetchFiles(projectId).catch(console.error); - }, 3000); + let inFlight = false; + const interval = setInterval(async () => { + if (inFlight) return; + inFlight = true; + try { await fetchFiles(projectId); } catch (e) { console.error(e); } + finally { inFlight = false; } + }, 3000);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
apps/client/package.json(2 hunks)apps/client/src/app/project/[projectId]/page.tsx(1 hunks)apps/client/src/components/agent/ChatPanel.tsx(1 hunks)apps/client/src/components/agent/FileSystemPanel.tsx(4 hunks)apps/client/src/components/agent/PreviewPanel.tsx(5 hunks)apps/client/src/store/project.ts(2 hunks)apps/server/package.json(3 hunks)apps/server/src/index.ts(2 hunks)apps/server/src/services/workspace.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
apps/client/src/components/agent/PreviewPanel.tsx (1)
apps/client/src/lib/utils.ts (1)
cn(4-6)
apps/server/src/index.ts (2)
apps/server/src/models/Project.ts (1)
Project(32-32)apps/server/src/services/workspace.ts (1)
WorkspaceService(7-120)
🔇 Additional comments (7)
apps/client/src/components/agent/ChatPanel.tsx (1)
51-60: Proxy endpoint switch looks good; ensure cookies/credentials expectations match
If the old API was cross-origin, verify the proxy forwards auth headers/cookies as intended.apps/client/src/app/project/[projectId]/page.tsx (1)
24-35: LGTM: proxy-based validation fetch
No concerns with the endpoint migration here.apps/server/src/index.ts (1)
13-14: Dependency usage: confirmarchiverAPI matches v7 semantics
Just ensure import style andarchive.finalize()behavior matchesarchiver@7.apps/client/src/components/agent/FileSystemPanel.tsx (1)
92-101: Download link wiring looks good
Proxy-relative href +rel="noopener noreferrer"is the right combo for_blank.apps/client/package.json (1)
17-23:canvas-confettiv1.9.4 and@types/canvas-confettiv1.9.0 are compatible and secure. No known security advisories exist for either package. Ensure the library is only called from client-side code (Client Components with "use client", inside useEffect, or via dynamic import withssr: false) to avoid SSR-related errors, since canvas-confetti depends on browser APIs (window/document/canvas).apps/server/package.json (1)
9-12:archiver@^7.0.1and@types/archiver@^7.0.0are compatible with your Node runtime and have no known security advisories.archiver 7.0.1 requires Node >=14, which is satisfied by your project's Node 18+ requirement. No direct CVEs exist for these npm packages; earlier versions had a transitive inflight vulnerability that 7.0.0+ resolves.
apps/client/src/components/agent/PreviewPanel.tsx (1)
68-74: Nice UX enhancement with confetti celebration.The confetti animation on preview ready state provides a satisfying user feedback. Configuration looks appropriate.
| // Download Route | ||
| app.get("/api/projects/:projectId/download", async (req, res) => { | ||
| try { | ||
| const { projectId } = req.params; | ||
| // Verify project exists | ||
| const project = await Project.findById(projectId); | ||
| if (!project) { | ||
| return res.status(404).json({ error: "Project not found" }); | ||
| } | ||
|
|
||
| const repoPath = WorkspaceService.getRepoPath(projectId); | ||
| if (!fs.existsSync(repoPath)) { | ||
| return res.status(404).json({ error: "No code generated yet" }); | ||
| } | ||
|
|
||
| res.setHeader('Content-Type', 'application/zip'); | ||
| res.setHeader('Content-Disposition', `attachment; filename="${project.name.replace(/[^a-z0-9]/gi, '_').toLowerCase() || 'project'}.zip"`); | ||
|
|
||
| const archive = archiver('zip', { | ||
| zlib: { level: 9 } // Sets the compression level. | ||
| }); | ||
|
|
||
| archive.on('error', function (err) { | ||
| console.error("Archiver error:", err); | ||
| res.status(500).send({ error: err.message }); | ||
| }); | ||
|
|
||
| // pipe archive data to the file | ||
| archive.pipe(res); | ||
|
|
||
| // append files from a sub-directory, putting its contents at the root of archive | ||
| archive.directory(repoPath, false); | ||
|
|
||
| archive.finalize(); | ||
|
|
||
| } catch (error) { | ||
| console.error("Download failed:", error); | ||
| res.status(500).json({ error: "Download failed" }); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Streaming error handling is unsafe after archive.pipe(res)
After piping, res.status(500).send(...) can throw or partially corrupt the zip. Prefer abort/destroy the stream and log.
- archive.on('error', function (err) {
- console.error("Archiver error:", err);
- res.status(500).send({ error: err.message });
- });
+ archive.on('error', (err) => {
+ console.error("Archiver error:", err);
+ // response is a stream at this point; don't try to write JSON/status reliably
+ res.destroy(err);
+ });
+
+ req.on('close', () => {
+ if (!res.writableEnded) archive.abort();
+ });There was a problem hiding this comment.
@M-DEV-1, thanks for the acknowledgment! Let me know if you'd like any clarification on the suggested approach for handling stream errors more safely.
M-DEV-1
changed the title
1 participant
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.