Skip to content

Commit 17c64ec

Browse files
nourzakhama2003nourzakhama2003
committed
apply feedback fix
1 parent 0d0d6a2 commit 17c64ec

6 files changed

Lines changed: 126 additions & 112 deletions

File tree

‎e2e-tests/github-import.spec.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -178,29 +178,29 @@ test("should allow empty commands to use defaults", async ({ po }) => {
178178
).not.toBeVisible();
179179
});
180180

181-
test("should auto-apply component tagger upgrade on GitHub import", async ({ po }) => {
181+
test("should auto-apply component tagger upgrade on GitHub import", async ({
182+
po,
183+
}) => {
182184
await po.setUp();
183185

184-
185186
await po.page.getByRole("button", { name: "Import App" }).click();
186187
await po.page.getByRole("tab", { name: "Your GitHub Repos" }).click();
187188
await po.page.getByRole("button", { name: "Connect to GitHub" }).click();
188189
await expect(po.page.locator("text=FAKE-CODE")).toBeVisible();
189190

190-
191191
await expect(po.page.getByText("testuser/existing-app")).toBeVisible();
192192
const repoRow = po.page.getByTestId("github-repo-row-testuser-existing-app");
193193
await expect(repoRow).toBeVisible();
194194
await repoRow.getByRole("button", { name: "Import" }).click();
195195

196-
197196
await expect(
198197
po.page.getByRole("heading", { name: "Import App" }),
199198
).not.toBeVisible();
200199

201-
202200
await po.appManagement.showAppList();
203-
await expect(po.appManagement.getAppListItem({ appName: "existing-app" })).toBeVisible({
201+
await expect(
202+
po.appManagement.getAppListItem({ appName: "existing-app" }),
203+
).toBeVisible({
204204
timeout: 20_000,
205205
});
206206
await po.appManagement.clickAppListItem({ appName: "existing-app" });

‎e2e-tests/helpers/page-objects/components/AppManagement.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,9 @@ export class AppManagement {
3131
return;
3232
}
3333

34-
const telemetryLaterButton = this.page.getByTestId("telemetry-later-button");
34+
const telemetryLaterButton = this.page.getByTestId(
35+
"telemetry-later-button",
36+
);
3537
if (await telemetryLaterButton.isVisible().catch(() => false)) {
3638
await telemetryLaterButton.click();
3739
}

‎src/ipc/handlers/app_upgrade_handlers.ts‎

Lines changed: 98 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,12 @@ import { getDyadAppPath } from "../../paths/paths";
88
import { DyadError, DyadErrorKind } from "@/errors/dyad_error";
99
import {
1010
isComponentTaggerUpgradeNeeded,
11-
isCapacitorUpgradeNeeded,
1211
applyComponentTagger,
13-
applyCapacitor,
1412
} from "../utils/app_upgrade_utils";
13+
import fs from "node:fs";
14+
import path from "node:path";
15+
import { spawn } from "node:child_process";
16+
import { gitAddAll, gitCommit } from "../utils/git_utils";
1517

1618
export const logger = log.scope("app_upgrade_handlers");
1719
const handle = createLoggedHandler(logger);
@@ -46,6 +48,100 @@ async function getApp(appId: number) {
4648
return app;
4749
}
4850

51+
function isViteApp(appPath: string): boolean {
52+
const viteConfigPathJs = path.join(appPath, "vite.config.js");
53+
const viteConfigPathTs = path.join(appPath, "vite.config.ts");
54+
55+
return fs.existsSync(viteConfigPathTs) || fs.existsSync(viteConfigPathJs);
56+
}
57+
58+
function isCapacitorUpgradeNeeded(appPath: string): boolean {
59+
// Check if it's a Vite app first
60+
if (!isViteApp(appPath)) {
61+
return false;
62+
}
63+
64+
// Check if Capacitor is already installed
65+
const capacitorConfigJs = path.join(appPath, "capacitor.config.js");
66+
const capacitorConfigTs = path.join(appPath, "capacitor.config.ts");
67+
const capacitorConfigJson = path.join(appPath, "capacitor.config.json");
68+
69+
// If any Capacitor config exists, the upgrade is not needed
70+
if (
71+
fs.existsSync(capacitorConfigJs) ||
72+
fs.existsSync(capacitorConfigTs) ||
73+
fs.existsSync(capacitorConfigJson)
74+
) {
75+
return false;
76+
}
77+
78+
return true;
79+
}
80+
81+
async function applyCapacitor({
82+
appName,
83+
appPath,
84+
}: {
85+
appName: string;
86+
appPath: string;
87+
}) {
88+
// Install Capacitor dependencies
89+
await simpleSpawn({
90+
command:
91+
"pnpm add @capacitor/core@7.4.4 @capacitor/cli@7.4.4 @capacitor/ios@7.4.4 @capacitor/android@7.4.4 || npm install @capacitor/core@7.4.4 @capacitor/cli@7.4.4 @capacitor/ios@7.4.4 @capacitor/android@7.4.4 --legacy-peer-deps",
92+
cwd: appPath,
93+
successMessage: "Capacitor dependencies installed successfully",
94+
errorPrefix: "Failed to install Capacitor dependencies",
95+
});
96+
97+
// Initialize Capacitor
98+
await simpleSpawn({
99+
command: `npx cap init "${appName}" "com.example.${appName.toLowerCase().replace(/[^a-z0-9]/g, "")}" --web-dir=dist`,
100+
cwd: appPath,
101+
successMessage: "Capacitor initialized successfully",
102+
errorPrefix: "Failed to initialize Capacitor",
103+
});
104+
105+
// Intentionally omit PNPM_INSTALL_POLICY_ARGS because:
106+
// 1. confirmModulesPurge will almost never be needed for capacitor (i.e. user would need to switch from npm to pnpm and not triggered a rebuild).
107+
// 2. strictBuildDeps should be kept true (default value) in case capacitor has native deps.
108+
await simpleSpawn({
109+
command:
110+
"pnpm install --prod=false || npm install --include=dev --legacy-peer-deps",
111+
cwd: appPath,
112+
successMessage: "Development dependencies installed successfully",
113+
errorPrefix: "Failed to install development dependencies",
114+
});
115+
116+
// Add iOS and Android platforms
117+
await simpleSpawn({
118+
command: "npx cap add ios && npx cap add android",
119+
cwd: appPath,
120+
successMessage: "iOS and Android platforms added successfully",
121+
errorPrefix: "Failed to add iOS and Android platforms",
122+
});
123+
124+
// Commit changes
125+
try {
126+
logger.info("Staging and committing Capacitor changes");
127+
await gitAddAll({ path: appPath });
128+
await gitCommit({
129+
path: appPath,
130+
message: "[dyad] add Capacitor for mobile app support",
131+
});
132+
logger.info("Successfully committed Capacitor changes");
133+
} catch (err) {
134+
logger.warn(
135+
`Failed to commit changes. This may happen if the project is not in a git repository, or if there are no changes to commit.`,
136+
err,
137+
);
138+
throw new Error(
139+
"Failed to commit Capacitor changes. Please commit them manually. Error: " +
140+
err,
141+
);
142+
}
143+
}
144+
49145
export function registerAppUpgradeHandlers() {
50146
handle(
51147
"get-app-upgrades",

‎src/ipc/handlers/github_handlers.ts‎

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -181,13 +181,13 @@ export async function prepareLocalBranch({
181181
if (isGitMergeInProgress({ path: appPath })) {
182182
throw new Error(
183183
"Cannot auto-commit changes because a merge is in progress. " +
184-
"Please complete or abort the merge and try again.",
184+
"Please complete or abort the merge and try again.",
185185
);
186186
}
187187
if (isGitRebaseInProgress({ path: appPath })) {
188188
throw new Error(
189189
"Cannot auto-commit changes because a rebase is in progress. " +
190-
"Please complete or abort the rebase and try again.",
190+
"Please complete or abort the rebase and try again.",
191191
);
192192
}
193193

@@ -262,8 +262,8 @@ export async function prepareLocalBranch({
262262
} catch (innerErr: any) {
263263
throw new Error(
264264
`Failed to resolve remote branch 'origin/${targetBranch}' to a commit. ` +
265-
"Ensure 'git fetch' succeeded and the remote branch exists. " +
266-
`${innerErr?.message || String(innerErr)}`,
265+
"Ensure 'git fetch' succeeded and the remote branch exists. " +
266+
`${innerErr?.message || String(innerErr)}`,
267267
);
268268
}
269269
}
@@ -288,7 +288,7 @@ export async function prepareLocalBranch({
288288
} else {
289289
logger.warn(
290290
"[GitHub Handler] Previous branch unknown; repository may remain in detached HEAD at " +
291-
`${commitSha}.`,
291+
`${commitSha}.`,
292292
);
293293
}
294294
throw error;
@@ -321,7 +321,7 @@ export async function prepareLocalBranch({
321321
) {
322322
throw new Error(
323323
`Failed to prepare local branch: uncommitted changes detected. ` +
324-
"Unable to automatically handle uncommitted changes. Please commit or stash your changes manually and try again.",
324+
"Unable to automatically handle uncommitted changes. Please commit or stash your changes manually and try again.",
325325
);
326326
}
327327
throw new Error(errorMessage);
@@ -437,8 +437,9 @@ async function pollForAccessToken(event: IpcMainInvokeEvent) {
437437
} catch (error) {
438438
logger.error("Error polling for GitHub access token:", error);
439439
event.sender.send("github:flow-error", {
440-
error: `Network or unexpected error during polling: ${error instanceof Error ? error.message : String(error)
441-
}`,
440+
error: `Network or unexpected error during polling: ${
441+
error instanceof Error ? error.message : String(error)
442+
}`,
442443
});
443444
stopPolling();
444445
}
@@ -1025,7 +1026,7 @@ export async function ensureCleanWorkspace(
10251026
if (isClean) return;
10261027
throw new Error(
10271028
`Workspace is not clean before ${operationDescription}. ` +
1028-
"Please commit or stash your changes manually and try again.",
1029+
"Please commit or stash your changes manually and try again.",
10291030
);
10301031
}
10311032

@@ -1156,7 +1157,7 @@ async function handleInviteCollaborator(
11561157
const data = (await response.json()) as { message?: string };
11571158
throw new Error(
11581159
data.message ||
1159-
`Failed to invite collaborator: ${response.status} ${response.statusText}`,
1160+
`Failed to invite collaborator: ${response.status} ${response.statusText}`,
11601161
);
11611162
}
11621163
} catch (err: any) {
@@ -1200,7 +1201,7 @@ async function handleRemoveCollaborator(
12001201
const data = (await response.json()) as { message?: string };
12011202
throw new Error(
12021203
data.message ||
1203-
`Failed to remove collaborator: ${response.status} ${response.statusText}`,
1204+
`Failed to remove collaborator: ${response.status} ${response.statusText}`,
12041205
);
12051206
}
12061207
} catch (err: any) {
@@ -1349,10 +1350,14 @@ async function handleCloneRepoFromUrl(
13491350
`Automatically applied component tagger upgrade for ${owner}/${repoName}`,
13501351
);
13511352
} catch (upgradeError) {
1352-
logger.warn(
1353+
logger.error(
13531354
`[GitHub Handler] Failed to auto-apply component tagger upgrade for ${owner}/${repoName}`,
13541355
upgradeError,
13551356
);
1357+
throw new DyadError(
1358+
`Failed to apply component tagger upgrade: ${upgradeError instanceof Error ? upgradeError.message : String(upgradeError)}`,
1359+
DyadErrorKind.External,
1360+
);
13561361
}
13571362
}
13581363

‎src/ipc/utils/app_upgrade_utils.ts‎

Lines changed: 1 addition & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -37,29 +37,6 @@ export function isComponentTaggerUpgradeNeeded(appPath: string): boolean {
3737
}
3838
}
3939

40-
export function isCapacitorUpgradeNeeded(appPath: string): boolean {
41-
// Check if it's a Vite app first
42-
if (!isViteApp(appPath)) {
43-
return false;
44-
}
45-
46-
// Check if Capacitor is already installed
47-
const capacitorConfigJs = path.join(appPath, "capacitor.config.js");
48-
const capacitorConfigTs = path.join(appPath, "capacitor.config.ts");
49-
const capacitorConfigJson = path.join(appPath, "capacitor.config.json");
50-
51-
// If any Capacitor config exists, the upgrade is not needed
52-
if (
53-
fs.existsSync(capacitorConfigJs) ||
54-
fs.existsSync(capacitorConfigTs) ||
55-
fs.existsSync(capacitorConfigJson)
56-
) {
57-
return false;
58-
}
59-
60-
return true;
61-
}
62-
6340
export async function applyComponentTagger(appPath: string) {
6441
const viteConfigPathJs = path.join(appPath, "vite.config.js");
6542
const viteConfigPathTs = path.join(appPath, "vite.config.ts");
@@ -111,7 +88,7 @@ export async function applyComponentTagger(appPath: string) {
11188
}
11289
} else {
11390
throw new Error(
114-
"Could not find `plugins: [` in vite.config.ts. Manual installation required.",
91+
`Could not find 'plugins: [' in ${path.basename(viteConfigPath)}. Manual installation required.`,
11592
);
11693
}
11794

@@ -164,67 +141,3 @@ export async function applyComponentTagger(appPath: string) {
164141
);
165142
}
166143
}
167-
168-
export async function applyCapacitor({
169-
appName,
170-
appPath,
171-
}: {
172-
appName: string;
173-
appPath: string;
174-
}) {
175-
// Install Capacitor dependencies
176-
await simpleSpawn({
177-
command:
178-
"pnpm add @capacitor/core@7.4.4 @capacitor/cli@7.4.4 @capacitor/ios@7.4.4 @capacitor/android@7.4.4 || npm install @capacitor/core@7.4.4 @capacitor/cli@7.4.4 @capacitor/ios@7.4.4 @capacitor/android@7.4.4 --legacy-peer-deps",
179-
cwd: appPath,
180-
successMessage: "Capacitor dependencies installed successfully",
181-
errorPrefix: "Failed to install Capacitor dependencies",
182-
});
183-
184-
// Initialize Capacitor
185-
await simpleSpawn({
186-
command: `npx cap init "${appName}" "com.example.${appName.toLowerCase().replace(/[^a-z0-9]/g, "")}" --web-dir=dist`,
187-
cwd: appPath,
188-
successMessage: "Capacitor initialized successfully",
189-
errorPrefix: "Failed to initialize Capacitor",
190-
});
191-
192-
// Intentionally omit PNPM_INSTALL_POLICY_ARGS because:
193-
// 1. confirmModulesPurge will almost never be needed for capacitor (i.e. user would need to switch from npm to pnpm and not triggered a rebuild).
194-
// 2. strictBuildDeps should be kept true (default value) in case capacitor has native deps.
195-
await simpleSpawn({
196-
command:
197-
"pnpm install --prod=false || npm install --include=dev --legacy-peer-deps",
198-
cwd: appPath,
199-
successMessage: "Development dependencies installed successfully",
200-
errorPrefix: "Failed to install development dependencies",
201-
});
202-
203-
// Add iOS and Android platforms
204-
await simpleSpawn({
205-
command: "npx cap add ios && npx cap add android",
206-
cwd: appPath,
207-
successMessage: "iOS and Android platforms added successfully",
208-
errorPrefix: "Failed to add iOS and Android platforms",
209-
});
210-
211-
// Commit changes
212-
try {
213-
logger.info("Staging and committing Capacitor changes");
214-
await gitAddAll({ path: appPath });
215-
await gitCommit({
216-
path: appPath,
217-
message: "[dyad] add Capacitor for mobile app support",
218-
});
219-
logger.info("Successfully committed Capacitor changes");
220-
} catch (err) {
221-
logger.warn(
222-
`Failed to commit changes. This may happen if the project is not in a git repository, or if there are no changes to commit.`,
223-
err,
224-
);
225-
throw new Error(
226-
"Failed to commit Capacitor changes. Please commit them manually. Error: " +
227-
err,
228-
);
229-
}
230-
}

‎testing/fake-llm-server/githubHandler.ts‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,7 @@ const mockUser = {
2626
email: "testuser@example.com",
2727
};
2828

29-
const mockReposRoot = fs.mkdtempSync(
30-
path.join(os.tmpdir(), "dyad-git-mock-"),
31-
);
29+
const mockReposRoot = fs.mkdtempSync(path.join(os.tmpdir(), "dyad-git-mock-"));
3230

3331
const mockRepos = [
3432
{

0 commit comments

Comments
 (0)