diff --git a/package-lock.json b/package-lock.json index f5a555b..34844e5 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1238,7 +1238,6 @@ "integrity": "sha512-GNWcUTRBgIRJD5zj+Tq0fKOJ5XZajIiBroOF0yvj2bSU1WvNdYS/dn9UxwsujGW4JX06dnHyjV2y9rRaybH0iQ==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "undici-types": "~7.16.0" } @@ -1289,7 +1288,6 @@ "integrity": "sha512-tK3GPFWbirvNgsNKto+UmB/cRtn6TZfyw0D6IKrW55n6Vbs7KJoZtI//kpTKzE/DUmmnAFD8/Ca46s7Obs92/w==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@typescript-eslint/scope-manager": "8.46.4", "@typescript-eslint/types": "8.46.4", @@ -1665,7 +1663,6 @@ "integrity": "sha512-NZyJarBfL7nWwIq+FDL6Zp/yHEhePMNnnJ0y3qfieCrmNvYct8uvtiV41UvlSe6apAfk0fY1FbWx+NwfmpvtTg==", "dev": true, "license": "MIT", - "peer": true, "bin": { "acorn": "bin/acorn" }, @@ -2017,7 +2014,6 @@ "integrity": "sha512-BhHmn2yNOFA9H9JmmIVKJmd288g9hrVRDkdoIgRCRuSySRUHH7r/DI6aAXW9T1WwUuY3DFgrcaqB+deURBLR5g==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@eslint-community/eslint-utils": "^4.8.0", "@eslint-community/regexpp": "^4.12.1", @@ -3631,7 +3627,6 @@ "integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==", "dev": true, "license": "MIT", - "peer": true, "engines": { "node": ">=12" }, @@ -3713,7 +3708,6 @@ "integrity": "sha512-jl1vZzPDinLr9eUt3J/t7V6FgNEw9QjvBPdysz9KfQDD41fQrC2Y4vKQdiaUpFT4bXlb1RHhLpp8wtm6M5TgSw==", "devOptional": true, "license": "Apache-2.0", - "peer": true, "bin": { "tsc": "bin/tsc", "tsserver": "bin/tsserver" @@ -3785,7 +3779,6 @@ "integrity": "sha512-BxAKBWmIbrDgrokdGZH1IgkIk/5mMHDreLDmCJ0qpyJaAteP8NvMhkwr/ZCQNqNH97bw/dANTE9PDzqwJghfMQ==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "esbuild": "^0.25.0", "fdir": "^6.5.0", @@ -3879,7 +3872,6 @@ "integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==", "dev": true, "license": "MIT", - "peer": true, "engines": { "node": ">=12" }, @@ -3893,7 +3885,6 @@ "integrity": "sha512-E0Ja2AX4th+CG33yAFRC+d1wFx2pzU5r6HtG6LiPSE04flaE0qB6YyjSw9ZcpJAtVPfsvZGtJlKWZpuW7EHRxg==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@vitest/expect": "4.0.9", "@vitest/mocker": "4.0.9", @@ -4196,12 +4187,10 @@ "license": "GPL-3.0-or-later", "dependencies": { "glob": "^11.0.3", + "tsx": "^4.20.6", "ums-lib": "^1.0.0", "yaml": "^2.6.0" }, - "optionalDependencies": { - "tsx": "^4.20.6" - }, "peerDependencies": { "typescript": ">=5.0.0" }, diff --git a/packages/ums-cli/src/utils/file-operations.ts b/packages/ums-cli/src/utils/file-operations.ts index 06cf087..fe28dfd 100644 --- a/packages/ums-cli/src/utils/file-operations.ts +++ b/packages/ums-cli/src/utils/file-operations.ts @@ -12,27 +12,38 @@ import { join } from 'path'; import { glob } from 'glob'; /** - * Reads a persona file and returns its content as a string + * Extracts error message from unknown error type */ -export async function readPersonaFile(path: string): Promise { +function getErrorMessage(error: unknown): string { + return error instanceof Error ? error.message : String(error); +} + +/** + * Generic file read operation with error handling + */ +async function readFileWithContext( + path: string, + context: string +): Promise { try { return await readFileAsync(path, 'utf-8'); } catch (error) { - const message = error instanceof Error ? error.message : String(error); - throw new Error(`Failed to read persona file '${path}': ${message}`); + throw new Error(`Failed to read ${context} '${path}': ${getErrorMessage(error)}`); } } +/** + * Reads a persona file and returns its content as a string + */ +export async function readPersonaFile(path: string): Promise { + return readFileWithContext(path, 'persona file'); +} + /** * Reads a module file and returns its content as a string */ export async function readModuleFile(path: string): Promise { - try { - return await readFileAsync(path, 'utf-8'); - } catch (error) { - const message = error instanceof Error ? error.message : String(error); - throw new Error(`Failed to read module file '${path}': ${message}`); - } + return readFileWithContext(path, 'module file'); } /** @@ -45,8 +56,7 @@ export async function writeOutputFile( try { await writeFileAsync(path, content, 'utf-8'); } catch (error) { - const message = error instanceof Error ? error.message : String(error); - throw new Error(`Failed to write output file '${path}': ${message}`); + throw new Error(`Failed to write output file '${path}': ${getErrorMessage(error)}`); } } @@ -66,9 +76,8 @@ export async function discoverModuleFiles(paths: string[]): Promise { allFiles.push(...files); } } catch (error) { - const message = error instanceof Error ? error.message : String(error); throw new Error( - `Failed to discover modules in path '${path}': ${message}` + `Failed to discover modules in path '${path}': ${getErrorMessage(error)}` ); } } diff --git a/packages/ums-sdk/src/loaders/loader-utils.test.ts b/packages/ums-sdk/src/loaders/loader-utils.test.ts new file mode 100644 index 0000000..be3de1e --- /dev/null +++ b/packages/ums-sdk/src/loaders/loader-utils.test.ts @@ -0,0 +1,108 @@ +/** + * Tests for loader utilities + */ + +import { describe, it, expect } from 'vitest'; +import { filePathToUrl, formatValidationErrors } from './loader-utils.js'; +import type { ValidationResult } from 'ums-lib'; + +describe('loader-utils', () => { + describe('filePathToUrl', () => { + it('should convert file path to file URL', () => { + const filePath = '/path/to/module.ts'; + const url = filePathToUrl(filePath); + + expect(url).toContain('file://'); + expect(url).toContain('module.ts'); + }); + + it('should handle paths with special characters', () => { + const filePath = '/path/to/my module.ts'; + const url = filePathToUrl(filePath); + + expect(url).toContain('file://'); + expect(url).toContain('my%20module.ts'); + }); + }); + + describe('formatValidationErrors', () => { + it('should format validation errors with paths', () => { + const validation: ValidationResult = { + valid: false, + errors: [ + { path: 'metadata.name', message: 'Name is required' }, + { path: 'components[0]', message: 'Invalid component' } + ], + warnings: [] + }; + + const result = formatValidationErrors(validation); + expect(result).toBe('metadata.name: Name is required; components[0]: Invalid component'); + }); + + it('should use default path for errors without path', () => { + const validation: ValidationResult = { + valid: false, + errors: [ + { message: 'Invalid structure' } + ], + warnings: [] + }; + + const result = formatValidationErrors(validation); + expect(result).toBe('module: Invalid structure'); + }); + + it('should use custom default path', () => { + const validation: ValidationResult = { + valid: false, + errors: [ + { message: 'Invalid structure' } + ], + warnings: [] + }; + + const result = formatValidationErrors(validation, 'persona'); + expect(result).toBe('persona: Invalid structure'); + }); + + it('should handle mixed errors with and without paths', () => { + const validation: ValidationResult = { + valid: false, + errors: [ + { path: 'id', message: 'ID is required' }, + { message: 'Missing schema version' }, + { path: 'version', message: 'Invalid version format' } + ], + warnings: [] + }; + + const result = formatValidationErrors(validation); + expect(result).toBe('id: ID is required; module: Missing schema version; version: Invalid version format'); + }); + + it('should handle single error', () => { + const validation: ValidationResult = { + valid: false, + errors: [ + { path: 'id', message: 'ID is required' } + ], + warnings: [] + }; + + const result = formatValidationErrors(validation); + expect(result).toBe('id: ID is required'); + }); + + it('should handle empty errors array', () => { + const validation: ValidationResult = { + valid: true, + errors: [], + warnings: [] + }; + + const result = formatValidationErrors(validation); + expect(result).toBe(''); + }); + }); +}); diff --git a/packages/ums-sdk/src/loaders/loader-utils.ts b/packages/ums-sdk/src/loaders/loader-utils.ts new file mode 100644 index 0000000..7b8d84b --- /dev/null +++ b/packages/ums-sdk/src/loaders/loader-utils.ts @@ -0,0 +1,30 @@ +/** + * Shared utilities for module and persona loaders + */ + +import { pathToFileURL } from 'node:url'; +import type { ValidationResult } from 'ums-lib'; + +/** + * Convert file path to file URL for dynamic import + * @param filePath - Absolute path to file + * @returns File URL string suitable for dynamic import + */ +export function filePathToUrl(filePath: string): string { + return pathToFileURL(filePath).href; +} + +/** + * Format validation errors into a single error message + * @param validation - Validation result from ums-lib + * @param defaultPath - Default path to use if error has no path (e.g., 'module', 'persona') + * @returns Formatted error message string + */ +export function formatValidationErrors( + validation: ValidationResult, + defaultPath = 'module' +): string { + return validation.errors + .map(e => `${e.path ?? defaultPath}: ${e.message}`) + .join('; '); +} diff --git a/packages/ums-sdk/src/loaders/module-loader.ts b/packages/ums-sdk/src/loaders/module-loader.ts index f93fa25..7a0a49f 100644 --- a/packages/ums-sdk/src/loaders/module-loader.ts +++ b/packages/ums-sdk/src/loaders/module-loader.ts @@ -13,7 +13,6 @@ */ import { readFile } from 'node:fs/promises'; -import { pathToFileURL } from 'node:url'; import { moduleIdToExportName, parseModule, @@ -25,7 +24,8 @@ import { ModuleNotFoundError, InvalidExportError, } from '../errors/index.js'; -import { checkFileExists } from '../utils/file-utils.js'; +import { checkFileExists, isFileNotFoundError } from '../utils/file-utils.js'; +import { filePathToUrl, formatValidationErrors } from './loader-utils.js'; /** * Checks if an object looks like a UMS Module (duck typing). @@ -66,7 +66,7 @@ export class ModuleLoader { await checkFileExists(filePath); // Convert file path to file URL for dynamic import - const fileUrl = pathToFileURL(filePath).href; + const fileUrl = filePathToUrl(filePath); // Dynamically import the TypeScript file (tsx handles compilation) const moduleExports = (await import(fileUrl)) as Record; @@ -120,11 +120,8 @@ export class ModuleLoader { // Delegate to ums-lib for full UMS v2.0 spec validation const validation = validateModule(parsedModule); if (!validation.valid) { - const errorMessages = validation.errors - .map(e => `${e.path ?? 'module'}: ${e.message}`) - .join('; '); throw new ModuleLoadError( - `Module validation failed: ${errorMessages}`, + `Module validation failed: ${formatValidationErrors(validation, 'module')}`, filePath ); } @@ -162,11 +159,8 @@ export class ModuleLoader { try { return await readFile(filePath, 'utf-8'); } catch (error) { - if (error && typeof error === 'object' && 'code' in error) { - const nodeError = error as NodeJS.ErrnoException; - if (nodeError.code === 'ENOENT') { - throw new ModuleNotFoundError(filePath); - } + if (isFileNotFoundError(error)) { + throw new ModuleNotFoundError(filePath); } throw new ModuleLoadError( `Failed to read file: ${error instanceof Error ? error.message : String(error)}`, diff --git a/packages/ums-sdk/src/loaders/persona-loader.ts b/packages/ums-sdk/src/loaders/persona-loader.ts index 4fb825b..94d0590 100644 --- a/packages/ums-sdk/src/loaders/persona-loader.ts +++ b/packages/ums-sdk/src/loaders/persona-loader.ts @@ -12,10 +12,10 @@ * - Validation (UMS v2.0 spec compliance) */ -import { pathToFileURL } from 'node:url'; import { parsePersona, validatePersona, type Persona } from 'ums-lib'; import { ModuleLoadError, ModuleNotFoundError } from '../errors/index.js'; import { checkFileExists } from '../utils/file-utils.js'; +import { filePathToUrl, formatValidationErrors } from './loader-utils.js'; /** * PersonaLoader - Loads and validates TypeScript persona files @@ -34,7 +34,7 @@ export class PersonaLoader { await checkFileExists(filePath); // Convert file path to file URL for dynamic import - const fileUrl = pathToFileURL(filePath).href; + const fileUrl = filePathToUrl(filePath); // Dynamically import the TypeScript file const personaExports = (await import(fileUrl)) as Record; @@ -69,11 +69,8 @@ export class PersonaLoader { // Delegate to ums-lib for full UMS v2.0 spec validation const validation = validatePersona(parsedPersona); if (!validation.valid) { - const errorMessages = validation.errors - .map(e => `${e.path ?? 'persona'}: ${e.message}`) - .join('; '); throw new ModuleLoadError( - `Persona validation failed: ${errorMessages}`, + `Persona validation failed: ${formatValidationErrors(validation, 'persona')}`, filePath ); } diff --git a/packages/ums-sdk/src/utils/file-utils.test.ts b/packages/ums-sdk/src/utils/file-utils.test.ts index b3e1ba1..c860b16 100644 --- a/packages/ums-sdk/src/utils/file-utils.test.ts +++ b/packages/ums-sdk/src/utils/file-utils.test.ts @@ -3,7 +3,7 @@ */ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; -import { checkFileExists } from './file-utils.js'; +import { checkFileExists, isFileNotFoundError } from './file-utils.js'; import { ModuleNotFoundError } from '../errors/index.js'; import * as fs from 'node:fs/promises'; @@ -14,6 +14,38 @@ vi.mock('node:fs/promises', () => ({ })); describe('file-utils', () => { + describe('isFileNotFoundError', () => { + it('should return true for ENOENT error', () => { + const error = Object.assign(new Error('ENOENT'), { code: 'ENOENT' }); + expect(isFileNotFoundError(error)).toBe(true); + }); + + it('should return false for other error codes', () => { + const error = Object.assign(new Error('EACCES'), { code: 'EACCES' }); + expect(isFileNotFoundError(error)).toBe(false); + }); + + it('should return false for errors without code property', () => { + const error = new Error('Some error'); + expect(isFileNotFoundError(error)).toBe(false); + }); + + it('should return false for errors with non-string code property', () => { + const error = Object.assign(new Error('Error'), { code: 123 }); + expect(isFileNotFoundError(error)).toBe(false); + }); + + it('should return false for null', () => { + expect(isFileNotFoundError(null)).toBe(false); + }); + + it('should return false for non-object errors', () => { + expect(isFileNotFoundError('string error')).toBe(false); + expect(isFileNotFoundError(123)).toBe(false); + expect(isFileNotFoundError(undefined)).toBe(false); + }); + }); + describe('checkFileExists', () => { beforeEach(() => { vi.clearAllMocks(); diff --git a/packages/ums-sdk/src/utils/file-utils.ts b/packages/ums-sdk/src/utils/file-utils.ts index 3b5e44b..4c68689 100644 --- a/packages/ums-sdk/src/utils/file-utils.ts +++ b/packages/ums-sdk/src/utils/file-utils.ts @@ -5,6 +5,25 @@ import { access, constants } from 'node:fs/promises'; import { ModuleNotFoundError } from '../errors/index.js'; +/** + * Type guard to check if an error is a NodeJS ErrnoException + */ +function isNodeError(error: unknown): error is NodeJS.ErrnoException { + return ( + error !== null && + typeof error === 'object' && + 'code' in error && + typeof (error as { code: unknown }).code === 'string' + ); +} + +/** + * Check if an error is an ENOENT (file not found) error + */ +export function isFileNotFoundError(error: unknown): boolean { + return isNodeError(error) && error.code === 'ENOENT'; +} + /** * Check if a file exists using access() for efficiency. * @param filePath - Absolute path to the file @@ -14,11 +33,8 @@ export async function checkFileExists(filePath: string): Promise { try { await access(filePath, constants.F_OK); } catch (error) { - if (error && typeof error === 'object' && 'code' in error) { - const nodeError = error as NodeJS.ErrnoException; - if (nodeError.code === 'ENOENT') { - throw new ModuleNotFoundError(filePath); - } + if (isFileNotFoundError(error)) { + throw new ModuleNotFoundError(filePath); } throw error; }