feat(desktop): replace Clone Repository with unified New Project dialog#1534
Closed
Kitenite wants to merge 1 commit into
Closed
feat(desktop): replace Clone Repository with unified New Project dialog#1534Kitenite wants to merge 1 commit into
Kitenite wants to merge 1 commit into
Conversation
Add a New Project dialog with three tabs (Empty, Clone, Template) replacing the single-purpose Clone Repository dialog. Extract shared initGitRepo helper and useProjectCreationHandler hook to eliminate duplication.
Contributor
📝 WalkthroughWalkthroughThis PR centralizes Git repository initialization logic into a reusable helper function and introduces a three-tab "New Project" dialog enabling creation of empty repositories, cloning existing ones, or scaffolding from templates. Two new TRPC procedures ( Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Dialog as NewProjectDialog
participant Tab as RepoTab Component
participant TRPC as TRPC Router
participant FS as File System
participant Git as Git
User->>Dialog: Opens new project dialog
User->>Tab: Selects tab & enters input
alt Empty Repo Flow
User->>Tab: Clicks "Create"
Tab->>TRPC: createEmptyRepo(name)
TRPC->>FS: Create folder
TRPC->>Git: Initialize repo
TRPC->>Git: Create initial commit
TRPC-->>Tab: Success + project
else Clone Flow
User->>Tab: Clicks "Clone"
Tab->>TRPC: cloneRepo(url)
TRPC->>Git: Clone repository
TRPC-->>Tab: Success + project
else Template Flow
User->>Tab: Selects/enters template URL
Tab->>TRPC: createFromTemplate(url)
TRPC->>Git: Clone template (depth=1)
TRPC->>Git: Strip history
TRPC->>Git: Initialize fresh repo
TRPC-->>Tab: Success + project
end
Tab->>Dialog: handleResult(project)
Dialog->>TRPC: createWorkspace(projectId)
Dialog->>User: Close modal
Dialog->>User: Show success/error toast
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (beta)
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
apps/desktop/src/renderer/screens/main/components/StartView/NewProjectDialog/hooks/useProjectCreationHandler/useProjectCreationHandler.ts (1)
27-30:resetStatecalled afteronClosemay target an unmounted component.
onClose()on Line 29 will close the dialog (and likely unmount the tab component that owns the state). TheresetState?.()on Line 30 then calls e.g.setName("")on unmounted state—effectively a no-op in React 19, but logically the ordering should be swapped.Proposed fix
if (result.success && result.project) { utils.projects.getRecents.invalidate(); createWorkspace.mutate({ projectId: result.project.id }); - onClose(); resetState?.(); + onClose(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/StartView/NewProjectDialog/hooks/useProjectCreationHandler/useProjectCreationHandler.ts` around lines 27 - 30, The order of teardown calls in useProjectCreationHandler is incorrect: calling onClose() before resetState may cause resetState to target an unmounted component; swap the two calls so resetState?.() runs first, then onClose(), while keeping utils.projects.getRecents.invalidate() and createWorkspace.mutate({ projectId: result.project.id }) unchanged to preserve side-effect order.apps/desktop/src/renderer/screens/main/components/StartView/NewProjectDialog/components/EmptyRepoTab/EmptyRepoTab.tsx (1)
20-34: Consider surfacing the name-validation regex client-side for faster feedback.The backend validates against
SAFE_REPO_NAME_REGEX(letters, numbers, dots, underscores, hyphens, spaces) but the frontend only checks for non-empty. Characters like/or..would pass the client check and round-trip to the backend before the user sees an error. A lightweight regex check here (or anaria-describedbyhint on the input) would improve UX.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/StartView/NewProjectDialog/components/EmptyRepoTab/EmptyRepoTab.tsx` around lines 20 - 34, Add client-side repo-name validation in handleCreate to mirror the backend SAFE_REPO_NAME_REGEX: before calling createEmptyRepo.mutate, run trimmed name against the same allowed-pattern (letters, numbers, dots, underscores, hyphens, spaces) and call onError with a clear message if it fails; keep the current empty check. Also add an input-level hint (e.g., aria-describedby on the name input) that displays the allowed-pattern so users get immediate feedback; ensure handleResult and setName behavior remains unchanged.apps/desktop/src/renderer/screens/main/components/StartView/NewProjectDialog/NewProjectDialog.tsx (2)
67-75: Tab content unmounts on switch — in-flight mutations will abort, and user input is lost.Since each tab is conditionally rendered (mount/unmount), switching tabs while a mutation is pending will abort it (due to
abortOnUnmount: trueon the tRPC client). Also, any partially entered form data is discarded. This is acceptable if intentional, but worth noting — if preserving draft input across tabs is desired, you'd need to lift state up or usedisplay: noneinstead of conditional rendering.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/StartView/NewProjectDialog/NewProjectDialog.tsx` around lines 67 - 75, Currently NewProjectDialog conditionally mounts EmptyRepoTab, CloneRepoTab, and TemplateRepoTab so switching tabs unmounts them (causing in-flight tRPC mutations to abort due to abortOnUnmount and losing form input). To fix, preserve tab components across switches by rendering all three and hiding inactive ones with CSS (e.g., apply a hidden class or style display: none) or lift per-tab form state into NewProjectDialog (or a shared parent/state store) so inputs and mutation state persist; update references to EmptyRepoTab, CloneRepoTab, TemplateRepoTab, and NewProjectDialog and ensure abortOnUnmount behavior is considered when choosing the approach.
55-59: Consider usingcn()for conditional class names for consistency.The tab buttons use a template literal for conditional styling, while
TemplateRepoTab(and likely other components) use thecn()utility from@superset/ui/utils. Usingcn()here would be more consistent and less error-prone for class merging.♻️ Suggested refactor
- className={`flex-1 px-3 py-1 text-xs font-medium rounded-sm transition-colors ${ - mode === tab.mode - ? "bg-background text-foreground shadow-sm" - : "text-muted-foreground hover:text-foreground" - }`} + className={cn( + "flex-1 px-3 py-1 text-xs font-medium rounded-sm transition-colors", + mode === tab.mode + ? "bg-background text-foreground shadow-sm" + : "text-muted-foreground hover:text-foreground", + )}This requires adding
cnto the imports:+import { cn } from "@superset/ui/utils";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/StartView/NewProjectDialog/NewProjectDialog.tsx` around lines 55 - 59, The tab button in NewProjectDialog.tsx uses a template literal for conditional classes (the expression comparing mode === tab.mode) instead of the project's cn() helper; update the JSX in the StartView/NewProjectDialog/NewProjectDialog component to import cn from `@superset/ui/utils` and replace the template literal with cn(baseClasses, { "bg-background text-foreground shadow-sm": mode === tab.mode, "text-muted-foreground hover:text-foreground": mode !== tab.mode }) so class merging is consistent with TemplateRepoTab and other components; ensure the import for cn is added to the file.apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarFooter.tsx (1)
122-126:NewProjectDialogis rendered in both branches — consider extracting it.The dialog JSX is duplicated in the collapsed (Line 122) and expanded (Line 161) branches. Since the dialog is a portal-based overlay and doesn't depend on the
isCollapsedlayout, it could be rendered once outside the conditional.♻️ Sketch: render dialog once
+ const newProjectDialog = ( + <NewProjectDialog + isOpen={isNewProjectDialogOpen} + onClose={() => setIsNewProjectDialogOpen(false)} + onError={handleNewProjectError} + /> + ); + if (isCollapsed) { return ( <> <div className="border-t border-border p-2 flex justify-center"> {/* ... dropdown ... */} </div> - <NewProjectDialog - isOpen={isNewProjectDialogOpen} - onClose={() => setIsNewProjectDialogOpen(false)} - onError={handleNewProjectError} - /> + {newProjectDialog} </> ); } return ( <> <div className="border-t border-border p-2"> {/* ... dropdown ... */} </div> - <NewProjectDialog - isOpen={isNewProjectDialogOpen} - onClose={() => setIsNewProjectDialogOpen(false)} - onError={handleNewProjectError} - /> + {newProjectDialog} </> );Also applies to: 161-165
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarFooter.tsx` around lines 122 - 126, The NewProjectDialog JSX is duplicated in both the collapsed/expanded branches; remove the duplicate and render a single instance of NewProjectDialog outside the isCollapsed conditional so it always mounts once. Keep using the existing props isNewProjectDialogOpen, onClose={() => setIsNewProjectDialogOpen(false)} and onError={handleNewProjectError} (leave setIsNewProjectDialogOpen and handleNewProjectError untouched) and delete the copies inside the branch that currently duplicate the dialog. Ensure no layout-specific props are passed to NewProjectDialog so it remains independent of isCollapsed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/src/lib/trpc/routers/projects/projects.ts`:
- Around line 928-962: The catch block for the create-from-template flow must
clean up the partially created directory to avoid leaving cloned files without
git metadata; update the error handling in the createFromTemplate flow so that
if any of initGitRepo or upsertProject (or other steps after clone and removing
.git) throw, you attempt to remove repoPath (use rm(repoPath, { recursive: true,
force: true }) and await it, swallowing any rm errors) before returning the
failure object; reference the cloned repoPath and the functions initGitRepo and
upsertProject when implementing the cleanup.
- Around line 806-861: The try block creates repoPath with mkdir then calls
initGitRepo and upsertProject; if either throws the empty directory is left
behind. Wrap the post-mkdir sequence (calls to initGitRepo, upsertProject,
ensureMainWorkspace, track) in its own try/catch and on any error remove the
newly created directory (await fs.rm(repoPath, { recursive: true, force: true })
or equivalent) before rethrowing or returning the failure result; use the
existing existsSync check to ensure you only remove directories you just created
and reference repoPath, mkdir, initGitRepo, upsertProject, and
ensureMainWorkspace to locate the code to change.
In
`@apps/desktop/src/renderer/screens/main/components/StartView/NewProjectDialog/constants.ts`:
- Around line 11-42: PROJECT_TEMPLATES contains GitHub browser subdirectory URLs
that are invalid for git.clone called by createFromTemplate (which uses
git.clone(input.templateUrl,...)); replace the three subdirectory URLs (Next.js,
Vite + React, Astro) with valid git clone targets or change the cloning
strategy. Either update the constants in PROJECT_TEMPLATES to full repository
clone URLs (e.g., use https://github.com/vercel/next.js.git,
https://github.com/vitejs/vite.git, https://github.com/withastro/astro.git) so
git.clone(templateUrl, ...) works, or modify createFromTemplate to use a tool
that supports subdirectory extraction (e.g., degit) and ensure git.clone
references (in createFromTemplate) are updated to use that tool and still read
input.templateUrl.
In
`@apps/desktop/src/renderer/screens/main/components/StartView/NewProjectDialog/hooks/useProjectCreationHandler/useProjectCreationHandler.ts`:
- Around line 24-34: The handler in useProjectCreationHandler currently ignores
cases where result.success is false but result.error is missing; update the
conditional so there is a final fallback branch that handles any non-success
outcome—e.g., after the existing if/else if, add an else that calls onError with
a generic Error or message (or triggers a user-facing notification), ensuring
you still invalidate recents/createWorkspace only on success; reference the
variables/result object and keep the existing calls to
utils.projects.getRecents.invalidate(), createWorkspace.mutate({ projectId:
result.project.id }), onClose(), and resetState?.() intact for the success path.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/screens/main/components/StartView/NewProjectDialog/components/EmptyRepoTab/EmptyRepoTab.tsx`:
- Around line 20-34: Add client-side repo-name validation in handleCreate to
mirror the backend SAFE_REPO_NAME_REGEX: before calling createEmptyRepo.mutate,
run trimmed name against the same allowed-pattern (letters, numbers, dots,
underscores, hyphens, spaces) and call onError with a clear message if it fails;
keep the current empty check. Also add an input-level hint (e.g.,
aria-describedby on the name input) that displays the allowed-pattern so users
get immediate feedback; ensure handleResult and setName behavior remains
unchanged.
In
`@apps/desktop/src/renderer/screens/main/components/StartView/NewProjectDialog/hooks/useProjectCreationHandler/useProjectCreationHandler.ts`:
- Around line 27-30: The order of teardown calls in useProjectCreationHandler is
incorrect: calling onClose() before resetState may cause resetState to target an
unmounted component; swap the two calls so resetState?.() runs first, then
onClose(), while keeping utils.projects.getRecents.invalidate() and
createWorkspace.mutate({ projectId: result.project.id }) unchanged to preserve
side-effect order.
In
`@apps/desktop/src/renderer/screens/main/components/StartView/NewProjectDialog/NewProjectDialog.tsx`:
- Around line 67-75: Currently NewProjectDialog conditionally mounts
EmptyRepoTab, CloneRepoTab, and TemplateRepoTab so switching tabs unmounts them
(causing in-flight tRPC mutations to abort due to abortOnUnmount and losing form
input). To fix, preserve tab components across switches by rendering all three
and hiding inactive ones with CSS (e.g., apply a hidden class or style display:
none) or lift per-tab form state into NewProjectDialog (or a shared parent/state
store) so inputs and mutation state persist; update references to EmptyRepoTab,
CloneRepoTab, TemplateRepoTab, and NewProjectDialog and ensure abortOnUnmount
behavior is considered when choosing the approach.
- Around line 55-59: The tab button in NewProjectDialog.tsx uses a template
literal for conditional classes (the expression comparing mode === tab.mode)
instead of the project's cn() helper; update the JSX in the
StartView/NewProjectDialog/NewProjectDialog component to import cn from
`@superset/ui/utils` and replace the template literal with cn(baseClasses, {
"bg-background text-foreground shadow-sm": mode === tab.mode,
"text-muted-foreground hover:text-foreground": mode !== tab.mode }) so class
merging is consistent with TemplateRepoTab and other components; ensure the
import for cn is added to the file.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarFooter.tsx`:
- Around line 122-126: The NewProjectDialog JSX is duplicated in both the
collapsed/expanded branches; remove the duplicate and render a single instance
of NewProjectDialog outside the isCollapsed conditional so it always mounts
once. Keep using the existing props isNewProjectDialogOpen, onClose={() =>
setIsNewProjectDialogOpen(false)} and onError={handleNewProjectError} (leave
setIsNewProjectDialogOpen and handleNewProjectError untouched) and delete the
copies inside the branch that currently duplicate the dialog. Ensure no
layout-specific props are passed to NewProjectDialog so it remains independent
of isCollapsed.
Comment on lines
+806
to
+861
| try { | ||
| const window = getWindow(); | ||
| if (!window) { | ||
| return { | ||
| canceled: false as const, | ||
| success: false as const, | ||
| error: "No window available", | ||
| }; | ||
| } | ||
|
|
||
| const result = await dialog.showOpenDialog(window, { | ||
| properties: ["openDirectory", "createDirectory"], | ||
| title: "Select Location for New Repository", | ||
| }); | ||
|
|
||
| if (result.canceled || result.filePaths.length === 0) { | ||
| return { canceled: true as const, success: false as const }; | ||
| } | ||
|
|
||
| const parentDir = result.filePaths[0]; | ||
| const repoPath = join(parentDir, input.name); | ||
|
|
||
| if (existsSync(repoPath)) { | ||
| return { | ||
| canceled: false as const, | ||
| success: false as const, | ||
| error: `A folder named "${input.name}" already exists at this location.`, | ||
| }; | ||
| } | ||
|
|
||
| await mkdir(repoPath, { recursive: true }); | ||
|
|
||
| const defaultBranch = await initGitRepo(repoPath); | ||
| const project = upsertProject(repoPath, defaultBranch); | ||
| await ensureMainWorkspace(project); | ||
|
|
||
| track("project_opened", { | ||
| project_id: project.id, | ||
| method: "create_empty", | ||
| }); | ||
|
|
||
| return { | ||
| canceled: false as const, | ||
| success: true as const, | ||
| project, | ||
| }; | ||
| } catch (error) { | ||
| const errorMessage = | ||
| error instanceof Error ? error.message : String(error); | ||
| return { | ||
| canceled: false as const, | ||
| success: false as const, | ||
| error: `Failed to create repository: ${errorMessage}`, | ||
| }; | ||
| } | ||
| }), |
Contributor
There was a problem hiding this comment.
No cleanup of created directory on failure.
If initGitRepo (Line 838) or upsertProject (Line 839) throws after mkdir (Line 836) succeeds, the empty directory is left on disk. The user sees an error but the stale folder will cause an "already exists" error on retry.
Consider wrapping the post-mkdir block in a try/catch that removes the directory on failure:
Proposed fix sketch
await mkdir(repoPath, { recursive: true });
+ try {
const defaultBranch = await initGitRepo(repoPath);
const project = upsertProject(repoPath, defaultBranch);
await ensureMainWorkspace(project);
track("project_opened", {
project_id: project.id,
method: "create_empty",
});
return {
canceled: false as const,
success: true as const,
project,
};
+ } catch (innerError) {
+ // Clean up partially-created directory
+ await rm(repoPath, { recursive: true, force: true }).catch(() => {});
+ throw innerError;
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/lib/trpc/routers/projects/projects.ts` around lines 806 -
861, The try block creates repoPath with mkdir then calls initGitRepo and
upsertProject; if either throws the empty directory is left behind. Wrap the
post-mkdir sequence (calls to initGitRepo, upsertProject, ensureMainWorkspace,
track) in its own try/catch and on any error remove the newly created directory
(await fs.rm(repoPath, { recursive: true, force: true }) or equivalent) before
rethrowing or returning the failure result; use the existing existsSync check to
ensure you only remove directories you just created and reference repoPath,
mkdir, initGitRepo, upsertProject, and ensureMainWorkspace to locate the code to
change.
Comment on lines
+928
to
+962
| // Clone the template repo (shallow), then strip its history | ||
| const git = simpleGit(); | ||
| await git.clone(input.templateUrl, repoPath, ["--depth", "1"]); | ||
| await rm(join(repoPath, ".git"), { | ||
| recursive: true, | ||
| force: true, | ||
| }); | ||
|
|
||
| const defaultBranch = await initGitRepo(repoPath, { | ||
| stageAll: true, | ||
| commitMessage: "Initial commit from template", | ||
| }); | ||
| const project = upsertProject(repoPath, defaultBranch); | ||
| await ensureMainWorkspace(project); | ||
|
|
||
| track("project_opened", { | ||
| project_id: project.id, | ||
| method: "create_from_template", | ||
| }); | ||
|
|
||
| return { | ||
| canceled: false as const, | ||
| success: true as const, | ||
| project, | ||
| }; | ||
| } catch (error) { | ||
| const errorMessage = | ||
| error instanceof Error ? error.message : String(error); | ||
| return { | ||
| canceled: false as const, | ||
| success: false as const, | ||
| error: `Failed to create project from template: ${errorMessage}`, | ||
| }; | ||
| } | ||
| }), |
Contributor
There was a problem hiding this comment.
Same cleanup concern applies to createFromTemplate.
If initGitRepo or upsertProject fails after the clone + .git removal (Lines 930-934), the directory is left in an inconsistent state (cloned files without git history). The same cleanup pattern would help here—remove repoPath on failure so the user can retry cleanly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/lib/trpc/routers/projects/projects.ts` around lines 928 -
962, The catch block for the create-from-template flow must clean up the
partially created directory to avoid leaving cloned files without git metadata;
update the error handling in the createFromTemplate flow so that if any of
initGitRepo or upsertProject (or other steps after clone and removing .git)
throw, you attempt to remove repoPath (use rm(repoPath, { recursive: true,
force: true }) and await it, swallowing any rm errors) before returning the
failure object; reference the cloned repoPath and the functions initGitRepo and
upsertProject when implementing the cleanup.
Comment on lines
+11
to
+42
| export const PROJECT_TEMPLATES: ProjectTemplate[] = [ | ||
| { | ||
| id: "nextjs", | ||
| name: "Next.js", | ||
| description: | ||
| "React framework with App Router, TypeScript, and Tailwind CSS", | ||
| url: "https://github.com/vercel/next.js/tree/canary/examples/hello-world", | ||
| tags: ["react", "typescript"], | ||
| }, | ||
| { | ||
| id: "vite-react", | ||
| name: "Vite + React", | ||
| description: "Fast React development with Vite, TypeScript, and ESLint", | ||
| url: "https://github.com/vitejs/vite/tree/main/packages/create-vite/template-react-ts", | ||
| tags: ["react", "typescript"], | ||
| }, | ||
| { | ||
| id: "astro", | ||
| name: "Astro", | ||
| description: | ||
| "Content-focused static site generator with island architecture", | ||
| url: "https://github.com/withastro/astro/tree/main/examples/minimal", | ||
| tags: ["static", "typescript"], | ||
| }, | ||
| { | ||
| id: "express", | ||
| name: "Express", | ||
| description: "Minimal Node.js web framework for APIs and web apps", | ||
| url: "https://github.com/expressjs/generator", | ||
| tags: ["node", "api"], | ||
| }, | ||
| ]; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for createFromTemplate in the codebase
echo "=== Searching for createFromTemplate implementation ==="
rg -n "createFromTemplate" --type=ts --type=tsx -A 10 -g '!node_modules' | head -100
echo ""
echo "=== Searching for project creation/clone logic ==="
rg -n "git clone|degit|clone.*template" --type=ts --type=tsx -g '!node_modules' -i | head -50
echo ""
echo "=== Looking for initGitRepo or related helpers ==="
rg -n "initGitRepo|git\(" --type=ts --type=tsx -g '!node_modules' -A 5 | head -100Repository: superset-sh/superset
Length of output: 324
🏁 Script executed:
#!/bin/bash
# Find where createFromTemplate is defined - likely in backend procedures/router
echo "=== Finding createFromTemplate definition ==="
fd -t f "router\|procedure" --type=ts --type=tsx -g '!node_modules' | xargs rg "createFromTemplate" -l
echo ""
echo "=== Search in apps/desktop/src/main ==="
find apps/desktop/src/main -name "*.ts" -o -name "*.tsx" | xargs rg "createFromTemplate" -A 15 2>/dev/null | head -150
echo ""
echo "=== Search in apps/backend or similar ==="
find . -path "*/backend/*" -name "*.ts" 2>/dev/null | xargs rg "createFromTemplate" -A 15 2>/dev/null | head -150Repository: superset-sh/superset
Length of output: 6329
🏁 Script executed:
#!/bin/bash
# Read the projects router file to see the full createFromTemplate implementation
cat -n apps/desktop/src/lib/trpc/routers/projects/projects.ts | head -300
echo ""
echo "=== Looking for ALLOWED_URL_PROTOCOLS and handler ==="
rg -n "ALLOWED_URL_PROTOCOLS|createFromTemplate.*=>" apps/desktop/src/lib/trpc/routers/projects/projects.ts -A 30Repository: superset-sh/superset
Length of output: 13937
🏁 Script executed:
#!/bin/bash
# Check the size of projects.ts first
wc -l apps/desktop/src/lib/trpc/routers/projects/projects.ts
# Get specific lines around createFromTemplate
sed -n '1,50p' apps/desktop/src/lib/trpc/routers/projects/projects.tsRepository: superset-sh/superset
Length of output: 1728
🏁 Script executed:
#!/bin/bash
# Get the createFromTemplate mutation handler implementation
sed -n '860,1000p' apps/desktop/src/lib/trpc/routers/projects/projects.tsRepository: superset-sh/superset
Length of output: 3726
🏁 Script executed:
#!/bin/bash
# Search for how the URL is actually used - look for git clone, degit, or other clone operations
rg -n "git.*clone|degit|clone.*repository|download.*template" apps/desktop/src/lib/trpc/routers/projects/projects.ts -A 10Repository: superset-sh/superset
Length of output: 1077
Three of four template URLs are GitHub subdirectory paths, not valid git clone targets.
The createFromTemplate mutation uses git.clone(input.templateUrl, ...) (line 930 of projects.ts), but the URLs for Next.js, Vite + React, and Astro use GitHub's /tree/ browser navigation paths (e.g., https://github.com/vercel/next.js/tree/canary/examples/hello-world). These are not valid git remote URLs — git clone will fail on all three templates at runtime.
Only the Express template points to a full repository and will work. Use full repository URLs instead, or switch to a tool like degit that supports subdirectory extraction.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/screens/main/components/StartView/NewProjectDialog/constants.ts`
around lines 11 - 42, PROJECT_TEMPLATES contains GitHub browser subdirectory
URLs that are invalid for git.clone called by createFromTemplate (which uses
git.clone(input.templateUrl,...)); replace the three subdirectory URLs (Next.js,
Vite + React, Astro) with valid git clone targets or change the cloning
strategy. Either update the constants in PROJECT_TEMPLATES to full repository
clone URLs (e.g., use https://github.com/vercel/next.js.git,
https://github.com/vitejs/vite.git, https://github.com/withastro/astro.git) so
git.clone(templateUrl, ...) works, or modify createFromTemplate to use a tool
that supports subdirectory extraction (e.g., degit) and ensure git.clone
references (in createFromTemplate) are updated to use that tool and still read
input.templateUrl.
Comment on lines
+24
to
+34
| ) => { | ||
| if (result.canceled) return; | ||
| if (result.success && result.project) { | ||
| utils.projects.getRecents.invalidate(); | ||
| createWorkspace.mutate({ projectId: result.project.id }); | ||
| onClose(); | ||
| resetState?.(); | ||
| } else if (!result.success && result.error) { | ||
| onError(result.error); | ||
| } | ||
| }; |
Contributor
There was a problem hiding this comment.
Silent no-op when success is false but error is absent.
If the mutation result has success: false with error being null or undefined, neither branch executes and the user sees no feedback. A trailing else with a generic fallback would be more resilient.
Proposed fix
if (result.canceled) return;
if (result.success && result.project) {
utils.projects.getRecents.invalidate();
createWorkspace.mutate({ projectId: result.project.id });
onClose();
resetState?.();
- } else if (!result.success && result.error) {
+ } else if (!result.success) {
- onError(result.error);
+ onError(result.error || "An unknown error occurred");
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ) => { | |
| if (result.canceled) return; | |
| if (result.success && result.project) { | |
| utils.projects.getRecents.invalidate(); | |
| createWorkspace.mutate({ projectId: result.project.id }); | |
| onClose(); | |
| resetState?.(); | |
| } else if (!result.success && result.error) { | |
| onError(result.error); | |
| } | |
| }; | |
| ) => { | |
| if (result.canceled) return; | |
| if (result.success && result.project) { | |
| utils.projects.getRecents.invalidate(); | |
| createWorkspace.mutate({ projectId: result.project.id }); | |
| onClose(); | |
| resetState?.(); | |
| } else if (!result.success) { | |
| onError(result.error || "An unknown error occurred"); | |
| } | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/screens/main/components/StartView/NewProjectDialog/hooks/useProjectCreationHandler/useProjectCreationHandler.ts`
around lines 24 - 34, The handler in useProjectCreationHandler currently ignores
cases where result.success is false but result.error is missing; update the
conditional so there is a final fallback branch that handles any non-success
outcome—e.g., after the existing if/else if, add an else that calls onError with
a generic Error or message (or triggers a user-facing notification), ensuring
you still invalidate recents/createWorkspace only on success; reference the
variables/result object and keep the existing calls to
utils.projects.getRecents.invalidate(), createWorkspace.mutate({ projectId:
result.project.id }), onClose(), and resetState?.() intact for the success path.
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
createEmptyRepoandcreateFromTemplatetRPC mutations to the projects routerinitGitRepobackend helper anduseProjectCreationHandlerfrontend hook to eliminate duplication across all project creation pathsWorkspaceSidebarFooterto use the new dialog as wellTest plan
bun run lint:fix && bun run typecheck— no errorsSummary by CodeRabbit