diff --git a/.changeset/bitter-rats-fail.md b/.changeset/bitter-rats-fail.md new file mode 100644 index 0000000..9de84b3 --- /dev/null +++ b/.changeset/bitter-rats-fail.md @@ -0,0 +1,5 @@ +--- +"scaffolder-toolkit": patch +--- + +fix(scaffolding): Remove partially created project directory on failure diff --git a/packages/devkit/TODO.md b/packages/devkit/TODO.md index ff964a6..69d39a4 100644 --- a/packages/devkit/TODO.md +++ b/packages/devkit/TODO.md @@ -66,7 +66,7 @@ This document tracks all planned and completed tasks for the Dev Kit project. - [x] add short keys to get config using `dk config` like `dk config lang` for `dk config language` - [x] ** Enhance for organization Purpose **: Add new language `Typescript(ts)` with same code as javascript(js), also support for nodejs(node) template name for those who prefer it than the programming language name - [x] **Testing**: Stabilize the integration test of the `new` command -- [ ] Make sure to clean up if the `dk new` command fail +- [x] Make sure to clean up if the `dk new` command fail - [ ] Add a configuration validation step when updating the config file to ensure all required fields are present and correctly formatted. - [ ] **Dynamic Help Text**: Programmatically generate help text for options with constrained values (e.g., `--cache-strategy`) to ensure it's always up to date. diff --git a/packages/devkit/__tests__/integrations/new.spec.ts b/packages/devkit/__tests__/integrations/new.spec.ts index ac96e4d..4b9920d 100644 --- a/packages/devkit/__tests__/integrations/new.spec.ts +++ b/packages/devkit/__tests__/integrations/new.spec.ts @@ -286,7 +286,6 @@ describe("dk new", () => { { all: true, cwd: tempDir }, ); expect(exitCode).toBe(0); - expect( await fs.pathExists(path.join(tempDir, "my-vue-app", "package.json")), ).toBe(true); diff --git a/packages/devkit/__tests__/units/scaffolding/javascript.spec.ts b/packages/devkit/__tests__/units/scaffolding/javascript.spec.ts index 398992b..e2d9570 100644 --- a/packages/devkit/__tests__/units/scaffolding/javascript.spec.ts +++ b/packages/devkit/__tests__/units/scaffolding/javascript.spec.ts @@ -6,98 +6,132 @@ import { import { DevkitError } from "../../../src/utils/errors/base.js"; import { mockSpinner, mockLogger } from "../../../vitest.setup.js"; -const { - mockRunCliCommand, - mockInstallDependencies, - mockCopyLocalTemplate, - mockGetTemplateFromCache, -} = vi.hoisted(() => ({ - mockRunCliCommand: vi.fn(), - mockInstallDependencies: vi.fn(), - mockGetTemplateFromCache: vi.fn(), - mockCopyLocalTemplate: vi.fn(), -})); - -vi.mock("#scaffolding/cli-runner.js", () => ({ - runCliCommand: mockRunCliCommand, -})); - -vi.mock("#core/cache/index.js", () => ({ - getTemplateFromCache: mockGetTemplateFromCache, -})); +const { mockScaffoldTemplate, mockInstallDependencies, mockFsRemove } = + vi.hoisted(() => ({ + mockScaffoldTemplate: vi.fn(), + mockInstallDependencies: vi.fn(), + mockFsRemove: vi.fn(), + })); -vi.mock("#scaffolding/local-template.js", () => ({ - copyLocalTemplate: mockCopyLocalTemplate, +vi.mock("../../../src/scaffolding/scaffold-template.js", () => ({ + scaffoldTemplate: mockScaffoldTemplate, })); vi.mock("#scaffolding/dependencies.js", () => ({ installDependencies: mockInstallDependencies, })); +vi.mock("#utils/fs/file.js", () => ({ + default: { + remove: mockFsRemove, + }, +})); + describe("scaffoldProject", () => { const options = { projectName: "my-project", packageManager: "npm", cacheStrategy: "daily", + templateConfig: { location: "mock-location" }, } as ScaffoldJavascriptProjectOptions; beforeEach(() => { vi.clearAllMocks(); + + mockScaffoldTemplate.mockResolvedValue({ + isOfficialCli: false, + projectDirCreated: false, + }); + mockInstallDependencies.mockResolvedValue(undefined); }); - it("should run official CLI command for a {pm} template", async () => { - const templateConfig = { location: "{pm} create vue" }; - await scaffoldProject({ ...options, templateConfig }); - expect(mockRunCliCommand).toHaveBeenCalledOnce(); - expect(mockRunCliCommand).toHaveBeenCalledWith({ - command: "{pm} create vue", - projectName: options.projectName, - packageManager: options.packageManager, - spinner: mockSpinner, + it("should skip dependency install and success log for official CLI templates", async () => { + mockScaffoldTemplate.mockResolvedValue({ + isOfficialCli: true, + projectDirCreated: false, }); + + await scaffoldProject(options); + + expect(mockScaffoldTemplate).toHaveBeenCalledOnce(); expect(mockInstallDependencies).not.toHaveBeenCalled(); - expect(mockSpinner.fail).not.toHaveBeenCalled(); + expect(mockLogger.log).not.toHaveBeenCalledWith( + expect.stringContaining("messages.success.scaffolding_complete"), + ); }); - it("should get template from cache for a remote Git URL", async () => { - const templateConfig = { location: "https://github.com/repo/test.git" }; - await scaffoldProject({ ...options, templateConfig }); - expect(mockGetTemplateFromCache).toHaveBeenCalledWith({ - url: templateConfig.location, - projectName: options.projectName, - spinner: mockSpinner, - strategy: options.cacheStrategy, + it("should run dependency install and log success for custom templates", async () => { + mockScaffoldTemplate.mockResolvedValue({ + isOfficialCli: false, + projectDirCreated: true, }); - expect(mockInstallDependencies).toHaveBeenCalled(); + + await scaffoldProject(options); + + expect(mockScaffoldTemplate).toHaveBeenCalledOnce(); + expect(mockInstallDependencies).toHaveBeenCalledOnce(); + expect(mockLogger.log).toHaveBeenCalledWith( + expect.stringContaining("messages.success.scaffolding_complete"), + ); }); - it("should copy local template for a relative path", async () => { - const templateConfig = { location: "./templates/local" }; - await scaffoldProject({ ...options, templateConfig }); - expect(mockCopyLocalTemplate).toHaveBeenCalledWith({ - sourcePath: templateConfig.location, - projectName: options.projectName, - spinner: mockSpinner, - }); - expect(mockInstallDependencies).toHaveBeenCalled(); + it("should NOT call fs.remove if projectDirCreated is false on failure", async () => { + mockScaffoldTemplate.mockRejectedValueOnce(new DevkitError("CLI error")); + + await scaffoldProject(options); + + expect(mockSpinner.fail).toHaveBeenCalledOnce(); + expect(mockFsRemove).not.toHaveBeenCalled(); + expect(mockLogger.error).toHaveBeenCalled(); }); - it("should call spinner.fail and console.error on any exception", async () => { - const templateConfig = { location: "http://invalid-url" }; - vi.mocked(mockGetTemplateFromCache).mockRejectedValueOnce( - new DevkitError("Test error"), + it("should call fs.remove and log cleanup warning if projectDirCreated is true on failure", async () => { + mockScaffoldTemplate.mockResolvedValueOnce({ + isOfficialCli: false, + projectDirCreated: true, + }); + vi.mocked(mockInstallDependencies).mockRejectedValueOnce( + new DevkitError("Install error"), ); - await scaffoldProject({ ...options, templateConfig }); - expect(mockSpinner.fail).toHaveBeenCalled(); + + await scaffoldProject(options); + + expect(mockSpinner.fail).toHaveBeenCalledOnce(); + expect(mockFsRemove).toHaveBeenCalledWith(options.projectName); expect(mockLogger.error).toHaveBeenCalled(); + expect(mockLogger.warning).toHaveBeenCalledWith( + expect.stringContaining("messages.status.project_removed"), + ); }); - it("should log success messages and next steps for non-CLI templates", async () => { - const templateConfig = { location: "http://example.com" }; - await scaffoldProject({ ...options, templateConfig }); - expect(mockInstallDependencies).toHaveBeenCalled(); - expect(mockLogger.log).toHaveBeenCalledWith( - expect.stringContaining("messages.success.scaffolding_complete"), + it("should log cleanup failure error if fs.remove itself fails", async () => { + mockScaffoldTemplate.mockResolvedValueOnce({ + isOfficialCli: false, + projectDirCreated: true, + }); + vi.mocked(mockInstallDependencies).mockRejectedValueOnce( + new DevkitError("Install error"), + ); + vi.mocked(mockFsRemove).mockRejectedValueOnce( + new Error("Permissions denied"), + ); + + await scaffoldProject(options); + + expect(mockSpinner.fail).toHaveBeenCalledOnce(); + expect(mockFsRemove).toHaveBeenCalledWith(options.projectName); + expect(mockLogger.error).toHaveBeenCalledTimes(2); + expect(mockLogger.error).toHaveBeenCalledWith( + expect.stringContaining("errors.scaffolding.fail"), + "CLEANUP", ); }); + + it("should call spinner.fail and console.error on any exception", async () => { + mockScaffoldTemplate.mockRejectedValueOnce(new Error("Generic error")); + await scaffoldProject(options); + expect(mockSpinner.fail).toHaveBeenCalled(); + expect(mockLogger.error).toHaveBeenCalled(); + expect(mockFsRemove).not.toHaveBeenCalled(); + }); }); diff --git a/packages/devkit/__tests__/units/scaffolding/scaffold-template.spec.ts b/packages/devkit/__tests__/units/scaffolding/scaffold-template.spec.ts new file mode 100644 index 0000000..bbd0758 --- /dev/null +++ b/packages/devkit/__tests__/units/scaffolding/scaffold-template.spec.ts @@ -0,0 +1,146 @@ +import { vi, describe, it, expect, beforeEach } from "vitest"; +import { scaffoldTemplate } from "../../../src/scaffolding/scaffold-template.js"; +import { DevkitError } from "../../../src/utils/errors/base.js"; +import { mockSpinner } from "../../../vitest.setup.js"; + +const { mockRunCliCommand, mockGetTemplateFromCache, mockCopyLocalTemplate } = + vi.hoisted(() => ({ + mockRunCliCommand: vi.fn(), + mockGetTemplateFromCache: vi.fn(), + mockCopyLocalTemplate: vi.fn(), + })); + +vi.mock("#scaffolding/cli-runner.js", () => ({ + runCliCommand: mockRunCliCommand, +})); + +vi.mock("#core/cache/index.js", () => ({ + getTemplateFromCache: mockGetTemplateFromCache, +})); + +vi.mock("#scaffolding/local-template.js", () => ({ + copyLocalTemplate: mockCopyLocalTemplate, +})); + +describe("scaffoldTemplate", () => { + const projectName = "my-test-app"; + const packageManager = "npm"; + const cacheStrategy = "daily"; + const spinner = mockSpinner; + + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("should run the official CLI command and return isOfficialCli: true", async () => { + const templateConfig = { location: "{pm} create vue" }; + + const result = await scaffoldTemplate( + projectName, + templateConfig, + packageManager, + cacheStrategy, + spinner, + ); + + expect(mockRunCliCommand).toHaveBeenCalledWith({ + command: templateConfig.location, + projectName, + packageManager, + spinner, + }); + expect(mockGetTemplateFromCache).not.toHaveBeenCalled(); + expect(mockCopyLocalTemplate).not.toHaveBeenCalled(); + + expect(result).toEqual({ isOfficialCli: true, projectDirCreated: false }); + + expect(spinner.text).not.toBe("messages.scaffolding.copy_start"); + expect(spinner.stop).toHaveBeenCalled(); + }); + + it("should get the template from cache and return projectDirCreated: true", async () => { + const templateConfig = { location: "https://github.com/repo/test.git" }; + + const result = await scaffoldTemplate( + projectName, + templateConfig, + packageManager, + cacheStrategy, + spinner, + ); + + expect(mockGetTemplateFromCache).toHaveBeenCalledWith({ + url: templateConfig.location, + projectName, + spinner, + strategy: cacheStrategy, + }); + expect(mockRunCliCommand).not.toHaveBeenCalled(); + expect(mockCopyLocalTemplate).not.toHaveBeenCalled(); + + expect(result).toEqual({ isOfficialCli: false, projectDirCreated: true }); + + expect(spinner.stop).not.toHaveBeenCalled(); + expect(spinner.succeed).not.toHaveBeenCalled(); + }); + + it("should get template from cache for a git@ url", async () => { + const templateConfig = { location: "git@github.com:repo/test.git" }; + + await scaffoldTemplate( + projectName, + templateConfig, + packageManager, + cacheStrategy, + spinner, + ); + + expect(mockGetTemplateFromCache).toHaveBeenCalledWith( + expect.objectContaining({ url: templateConfig.location }), + ); + }); + + it("should copy the local template and return projectDirCreated: true", async () => { + const templateConfig = { location: "./templates/local" }; + + const result = await scaffoldTemplate( + projectName, + templateConfig, + packageManager, + cacheStrategy, + spinner, + ); + + expect(mockCopyLocalTemplate).toHaveBeenCalledWith({ + sourcePath: templateConfig.location, + projectName, + spinner, + }); + expect(mockRunCliCommand).not.toHaveBeenCalled(); + expect(mockGetTemplateFromCache).not.toHaveBeenCalled(); + + expect(result).toEqual({ isOfficialCli: false, projectDirCreated: true }); + + expect(spinner.text).toBe("messages.scaffolding.copy_start"); + expect(spinner.succeed).toHaveBeenCalled(); + }); + + it("should throw an exception if any underlying scaffolding call fails", async () => { + const templateConfig = { location: "http://invalid-url" }; + const error = new DevkitError("Cache download failed"); + + vi.mocked(mockGetTemplateFromCache).mockRejectedValueOnce(error); + + await expect( + scaffoldTemplate( + projectName, + templateConfig, + packageManager, + cacheStrategy, + spinner, + ), + ).rejects.toThrow(error); + + expect(spinner.fail).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/devkit/locales/en.json b/packages/devkit/locales/en.json index ca4c91f..51120ee 100644 --- a/packages/devkit/locales/en.json +++ b/packages/devkit/locales/en.json @@ -268,7 +268,8 @@ "config_source_local": "Using local configuration.", "config_source_global": "Using global configuration.", "templates_using_global_fallback": "No local configuration found. Using templates from global configuration.", - "including_defaults_suffix": "(including default templates)" + "including_defaults_suffix": "(including default templates)", + "project_removed": "Project '{project}' removed successfully." }, "scaffolding": { "start": "Scaffolding {language} project: {project}", diff --git a/packages/devkit/locales/fr.json b/packages/devkit/locales/fr.json index 1a0f8d6..b83ce8a 100644 --- a/packages/devkit/locales/fr.json +++ b/packages/devkit/locales/fr.json @@ -268,7 +268,8 @@ "config_source_local": "Utilisation de la configuration locale.", "config_source_global": "Utilisation de la configuration globale.", "templates_using_global_fallback": "Aucune configuration locale trouvée. Utilisation des modèles de la configuration globale.", - "including_defaults_suffix": "(incluant les modèles par défaut)" + "including_defaults_suffix": "(incluant les modèles par défaut)", + "project_removed": "Projet '{project}' supprimé avec succès." }, "scaffolding": { "start": "Échafaudage du projet {language} : {project}", diff --git a/packages/devkit/src/core/template/update-project-name.ts b/packages/devkit/src/core/template/update-project-name.ts index bf44238..8aa3027 100644 --- a/packages/devkit/src/core/template/update-project-name.ts +++ b/packages/devkit/src/core/template/update-project-name.ts @@ -17,9 +17,11 @@ export async function updateJavascriptProjectName( try { const packageJson = await fs.readJson(packageJsonPath); - packageJson.name = newProjectName; - await fs.writeJson(packageJsonPath, packageJson); + await fs.writeJson(packageJsonPath, { + ...packageJson, + name: newProjectName, + }); } catch (error) { const errorMessage = t("errors.system.package_name_update_fail"); diff --git a/packages/devkit/src/scaffolding/javascript.ts b/packages/devkit/src/scaffolding/javascript.ts index 4c0b4af..ab55caa 100644 --- a/packages/devkit/src/scaffolding/javascript.ts +++ b/packages/devkit/src/scaffolding/javascript.ts @@ -1,9 +1,8 @@ import { t } from "#utils/i18n/translator.js"; -import { getTemplateFromCache } from "#core/cache/index.js"; -import { runCliCommand } from "#scaffolding/cli-runner.js"; -import { copyLocalTemplate } from "#scaffolding/local-template.js"; import { installDependencies } from "#scaffolding/dependencies.js"; import { logger } from "#utils/logger.js"; +import fs from "#utils/fs/file.js"; +import { scaffoldTemplate } from "./scaffold-template.js"; import type { TemplateConfig, CacheStrategy, @@ -17,53 +16,48 @@ export interface ScaffoldJavascriptProjectOptions { cacheStrategy: CacheStrategy; } +function logSuccessMessages(projectName: string) { + logger.log( + logger.colors.green( + logger.colors.bold(t("messages.success.scaffolding_complete")), + ), + ); + logger.log( + logger.colors.white( + logger.colors.bold( + logger.colors.italic(t("messages.success.next_steps")), + ), + ), + ); + logger.log( + logger.colors.green( + logger.colors.bold( + ` cd ${projectName}\n git init && git add -A && git commit -m "Initial commit"\n`, + ), + ), + ); +} + export async function scaffoldProject( options: ScaffoldJavascriptProjectOptions, ): Promise { const { projectName, templateConfig, packageManager, cacheStrategy } = options; const spinner = logger.spinner(t("messages.status.scaffolding_project")); + let projectDirCreated = false; let isOfficialCli = false; try { - if (templateConfig.location.includes("{pm}")) { - isOfficialCli = true; - spinner.text = logger.colors.cyan( - logger.colors.bold( - t("messages.scaffolding.run_start", { - command: templateConfig.location, - }), - ), - ); - spinner.stop(); - await runCliCommand({ - command: templateConfig.location, - projectName, - packageManager, - spinner, - }); - } else if ( - templateConfig.location.startsWith("http") || - templateConfig.location.startsWith("git@") - ) { - await getTemplateFromCache({ - url: templateConfig.location, - projectName, - spinner, - strategy: cacheStrategy, - }); - } else { - spinner.text = logger.colors.cyan(t("messages.scaffolding.copy_start")); - spinner.start(); - await copyLocalTemplate({ - sourcePath: templateConfig.location, - projectName, - spinner, - }); - spinner.succeed( - logger.colors.green(t("messages.scaffolding.copy_success")), - ); - } + spinner.start(); + const result = await scaffoldTemplate( + projectName, + templateConfig, + packageManager, + cacheStrategy, + spinner, + ); + isOfficialCli = result.isOfficialCli; + projectDirCreated = result.projectDirCreated; if (!isOfficialCli) { spinner.text = logger.colors.cyan( @@ -76,32 +70,31 @@ export async function scaffoldProject( } if (!isOfficialCli) { - logger.log( - logger.colors.green( - logger.colors.bold(t("messages.success.scaffolding_complete")), - ), - ); - logger.log( - logger.colors.white( - logger.colors.bold( - logger.colors.italic(t("messages.success.next_steps")), - ), - ), - ); - logger.log( - logger.colors.green( - logger.colors.bold( - ` cd ${projectName}\n git init && git add -A && git commit -m "Initial commit"\n`, - ), - ), - ); + logSuccessMessages(projectName); } } catch (err: unknown) { spinner.fail(logger.colors.red(t("errors.scaffolding.unexpected"))); + if (projectDirCreated) { + try { + await fs.remove(projectName); + logger.warning( + logger.colors.yellow( + t("messages.status.project_removed", { project: projectName }), + ), + ); + // oxlint-disable-next-line no-unused-vars + } catch (cleanupErr: unknown) { + logger.error( + t("errors.scaffolding.fail", { project: projectName }), + "CLEANUP", + ); + } + } + const message = t("errors.scaffolding.unexpected"); if (err instanceof Error) { - logger.error(`${message}: ${err.message}`, "UNKNOWN"); + logger.error(`${message}: ${err.cause}`, "UNKNOWN"); } else { logger.error(message, "UNKNOWN"); } diff --git a/packages/devkit/src/scaffolding/scaffold-template.ts b/packages/devkit/src/scaffolding/scaffold-template.ts new file mode 100644 index 0000000..d1824cf --- /dev/null +++ b/packages/devkit/src/scaffolding/scaffold-template.ts @@ -0,0 +1,63 @@ +import { t } from "#utils/i18n/translator.js"; +import { getTemplateFromCache } from "#core/cache/index.js"; +import { runCliCommand } from "#scaffolding/cli-runner.js"; +import { copyLocalTemplate } from "#scaffolding/local-template.js"; +import { logger } from "#utils/logger.js"; +import type { + TemplateConfig, + CacheStrategy, + SupportedJavascriptPackageManager, +} from "#utils/schema/schema.js"; + +interface TemplateScaffoldingResult { + isOfficialCli: boolean; + projectDirCreated: boolean; +} + +export async function scaffoldTemplate( + projectName: string, + templateConfig: TemplateConfig, + packageManager: SupportedJavascriptPackageManager, + cacheStrategy: CacheStrategy, + spinner: ReturnType, +): Promise { + const { location } = templateConfig; + + if (location.includes("{pm}")) { + spinner.text = logger.colors.cyan( + logger.colors.bold( + t("messages.scaffolding.run_start", { + command: location, + }), + ), + ); + spinner.stop(); + await runCliCommand({ + command: location, + projectName, + packageManager, + spinner, + }); + return { isOfficialCli: true, projectDirCreated: false }; + } else if (location.startsWith("http") || location.startsWith("git@")) { + await getTemplateFromCache({ + url: location, + projectName, + spinner, + strategy: cacheStrategy, + }); + return { isOfficialCli: false, projectDirCreated: true }; + } else { + spinner.text = logger.colors.cyan(t("messages.scaffolding.copy_start")); + spinner.start(); + await copyLocalTemplate({ + sourcePath: location, + projectName, + spinner, + }); + spinner.succeed( + logger.colors.green(t("messages.scaffolding.copy_success")), + ); + return { isOfficialCli: false, projectDirCreated: true }; + } +} diff --git a/packages/devkit/src/utils/logger.ts b/packages/devkit/src/utils/logger.ts index 09ceadc..7feec62 100644 --- a/packages/devkit/src/utils/logger.ts +++ b/packages/devkit/src/utils/logger.ts @@ -77,6 +77,7 @@ export type ErrorType = | "ERR" | "CONFIG" | "TEMPL" + | "CLEANUP" | "CACHE"; export type TSpinner = Ora;