fix(#1): Address code review findings
- Convert ApiResponse to discriminated union for type-safe error handling - Add HealthStatus type with HealthState literal union - Make BaseEntity fields readonly for immutability - Add GlobalExceptionFilter with structured logging - Add port validation with clear error messages in main.ts - Improve parseDate to log warnings for invalid dates - Add comprehensive Button tests (variants, onClick, disabled) - Add slugify edge case tests (empty, special chars, numbers) - Create ESLint configs for all packages - Remove compiled JS files from src directories - Convert .prettierrc.js to .prettierrc.json Refs #1 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -4,9 +4,9 @@ export default [
|
||||
...baseConfig,
|
||||
{
|
||||
rules: {
|
||||
"@typescript-eslint/interface-name-prefix": "off",
|
||||
"@typescript-eslint/explicit-function-return-type": "off",
|
||||
"@typescript-eslint/explicit-module-boundary-types": "off",
|
||||
"@typescript-eslint/no-extraneous-class": "off",
|
||||
},
|
||||
},
|
||||
];
|
||||
|
||||
16
packages/shared/eslint.config.js
Normal file
16
packages/shared/eslint.config.js
Normal file
@@ -0,0 +1,16 @@
|
||||
import baseConfig from "@mosaic/config/eslint/base";
|
||||
|
||||
export default [
|
||||
...baseConfig,
|
||||
{
|
||||
languageOptions: {
|
||||
parserOptions: {
|
||||
project: ["./tsconfig.json"],
|
||||
tsconfigRootDir: import.meta.dirname,
|
||||
},
|
||||
},
|
||||
},
|
||||
{
|
||||
ignores: ["dist/**", "**/*.test.ts", "**/*.spec.ts"],
|
||||
},
|
||||
];
|
||||
@@ -1,38 +1,121 @@
|
||||
/**
|
||||
* Base entity type with common fields
|
||||
* @invariant updatedAt >= createdAt
|
||||
*/
|
||||
export interface BaseEntity {
|
||||
id: string;
|
||||
createdAt: Date;
|
||||
readonly id: string;
|
||||
readonly createdAt: Date;
|
||||
updatedAt: Date;
|
||||
}
|
||||
|
||||
/**
|
||||
* API response wrapper
|
||||
* API response wrapper - discriminated union for type-safe error handling
|
||||
*
|
||||
* Usage:
|
||||
* ```typescript
|
||||
* if (response.success) {
|
||||
* // TypeScript knows response.data exists
|
||||
* console.log(response.data);
|
||||
* } else {
|
||||
* // TypeScript knows response.message exists
|
||||
* console.error(response.message);
|
||||
* }
|
||||
* ```
|
||||
*/
|
||||
export interface ApiResponse<T> {
|
||||
data: T;
|
||||
success: boolean;
|
||||
message?: string;
|
||||
export type ApiResponse<T> =
|
||||
| { success: true; data: T; message?: string }
|
||||
| { success: false; data?: never; message: string; errorCode?: string };
|
||||
|
||||
/**
|
||||
* Helper to create a successful API response
|
||||
*/
|
||||
export function successResponse<T>(data: T, message?: string): ApiResponse<T> {
|
||||
if (message !== undefined) {
|
||||
return { success: true, data, message };
|
||||
}
|
||||
return { success: true, data };
|
||||
}
|
||||
|
||||
/**
|
||||
* Helper to create an error API response
|
||||
*/
|
||||
export function errorResponse<T>(message: string, errorCode?: string): ApiResponse<T> {
|
||||
if (errorCode !== undefined) {
|
||||
return { success: false, message, errorCode };
|
||||
}
|
||||
return { success: false, message };
|
||||
}
|
||||
|
||||
/**
|
||||
* Pagination parameters
|
||||
*/
|
||||
export interface PaginationParams {
|
||||
export interface PaginationParams<SortableFields extends string = string> {
|
||||
/** Page number (1-indexed) */
|
||||
page: number;
|
||||
/** Items per page */
|
||||
limit: number;
|
||||
sortBy?: string;
|
||||
/** Field to sort by */
|
||||
sortBy?: SortableFields;
|
||||
/** Sort direction */
|
||||
sortOrder?: "asc" | "desc";
|
||||
}
|
||||
|
||||
/**
|
||||
* Paginated response
|
||||
* @invariant totalPages === Math.ceil(total / limit)
|
||||
* @invariant data.length <= limit
|
||||
* @invariant page >= 1 && page <= totalPages (when total > 0)
|
||||
*/
|
||||
export interface PaginatedResponse<T> {
|
||||
data: T[];
|
||||
total: number;
|
||||
page: number;
|
||||
limit: number;
|
||||
totalPages: number;
|
||||
readonly data: readonly T[];
|
||||
readonly total: number;
|
||||
readonly page: number;
|
||||
readonly limit: number;
|
||||
readonly totalPages: number;
|
||||
}
|
||||
|
||||
/**
|
||||
* Helper to create a paginated response with calculated totalPages
|
||||
*/
|
||||
export function createPaginatedResponse<T>(
|
||||
data: T[],
|
||||
total: number,
|
||||
page: number,
|
||||
limit: number
|
||||
): PaginatedResponse<T> {
|
||||
if (limit <= 0) {
|
||||
throw new Error("limit must be positive");
|
||||
}
|
||||
if (page < 1) {
|
||||
throw new Error("page must be >= 1");
|
||||
}
|
||||
|
||||
return {
|
||||
data,
|
||||
total,
|
||||
page,
|
||||
limit,
|
||||
totalPages: Math.ceil(total / limit) || 1,
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Health check status values
|
||||
*/
|
||||
export type HealthState = "healthy" | "degraded" | "unhealthy";
|
||||
|
||||
/**
|
||||
* Health check response
|
||||
*/
|
||||
export interface HealthStatus {
|
||||
status: HealthState;
|
||||
timestamp: string;
|
||||
version?: string;
|
||||
checks?: Record<
|
||||
string,
|
||||
{
|
||||
status: HealthState;
|
||||
message?: string;
|
||||
}
|
||||
>;
|
||||
}
|
||||
|
||||
@@ -35,6 +35,38 @@ describe("slugify", () => {
|
||||
it("should trim leading and trailing hyphens", () => {
|
||||
expect(slugify(" Hello World ")).toBe("hello-world");
|
||||
});
|
||||
|
||||
it("should handle empty string", () => {
|
||||
expect(slugify("")).toBe("");
|
||||
});
|
||||
|
||||
it("should handle string with only whitespace", () => {
|
||||
expect(slugify(" ")).toBe("");
|
||||
});
|
||||
|
||||
it("should handle consecutive special characters", () => {
|
||||
expect(slugify("Hello---World")).toBe("hello-world");
|
||||
expect(slugify("Hello___World")).toBe("hello-world");
|
||||
expect(slugify("Hello World")).toBe("hello-world");
|
||||
});
|
||||
|
||||
it("should handle numbers", () => {
|
||||
expect(slugify("Product 123")).toBe("product-123");
|
||||
expect(slugify("2024 New Year")).toBe("2024-new-year");
|
||||
});
|
||||
|
||||
it("should handle mixed case", () => {
|
||||
expect(slugify("HeLLo WoRLd")).toBe("hello-world");
|
||||
});
|
||||
|
||||
it("should remove leading/trailing special chars", () => {
|
||||
expect(slugify("---Hello World---")).toBe("hello-world");
|
||||
expect(slugify("!@#Hello World!@#")).toBe("hello-world");
|
||||
});
|
||||
|
||||
it("should handle string with only special characters", () => {
|
||||
expect(slugify("!@#$%^&*()")).toBe("");
|
||||
});
|
||||
});
|
||||
|
||||
describe("sleep", () => {
|
||||
|
||||
@@ -1,11 +1,16 @@
|
||||
/**
|
||||
* Safely parse a date string or return undefined
|
||||
* Safely parse a date string or return undefined.
|
||||
* Logs a warning if an invalid date string is passed.
|
||||
*/
|
||||
export function parseDate(value: string | Date | undefined | null): Date | undefined {
|
||||
if (!value) return undefined;
|
||||
if (value instanceof Date) return value;
|
||||
const parsed = new Date(value);
|
||||
return isNaN(parsed.getTime()) ? undefined : parsed;
|
||||
if (isNaN(parsed.getTime())) {
|
||||
console.warn(`parseDate: Invalid date string received: "${value}"`);
|
||||
return undefined;
|
||||
}
|
||||
return parsed;
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
16
packages/ui/eslint.config.js
Normal file
16
packages/ui/eslint.config.js
Normal file
@@ -0,0 +1,16 @@
|
||||
import baseConfig from "@mosaic/config/eslint/base";
|
||||
|
||||
export default [
|
||||
...baseConfig,
|
||||
{
|
||||
languageOptions: {
|
||||
parserOptions: {
|
||||
project: ["./tsconfig.json"],
|
||||
tsconfigRootDir: import.meta.dirname,
|
||||
},
|
||||
},
|
||||
},
|
||||
{
|
||||
ignores: ["dist/**", "**/*.test.tsx", "**/*.test.ts", "**/*.spec.tsx", "**/*.spec.ts"],
|
||||
},
|
||||
];
|
||||
@@ -1,5 +1,5 @@
|
||||
import { describe, expect, it, afterEach } from "vitest";
|
||||
import { render, screen, cleanup } from "@testing-library/react";
|
||||
import { describe, expect, it, afterEach, vi } from "vitest";
|
||||
import { render, screen, cleanup, fireEvent } from "@testing-library/react";
|
||||
import { Button } from "./Button.js";
|
||||
|
||||
afterEach(() => {
|
||||
@@ -12,20 +12,75 @@ describe("Button", () => {
|
||||
expect(screen.getByRole("button")).toHaveTextContent("Click me");
|
||||
});
|
||||
|
||||
it("should apply variant styles", () => {
|
||||
render(<Button variant="danger">Delete</Button>);
|
||||
const button = screen.getByRole("button");
|
||||
expect(button.className).toContain("bg-red-600");
|
||||
describe("variants", () => {
|
||||
it("should apply primary variant styles by default", () => {
|
||||
render(<Button>Primary</Button>);
|
||||
const button = screen.getByRole("button");
|
||||
expect(button.className).toContain("bg-blue-600");
|
||||
});
|
||||
|
||||
it("should apply secondary variant styles", () => {
|
||||
render(<Button variant="secondary">Secondary</Button>);
|
||||
const button = screen.getByRole("button");
|
||||
expect(button.className).toContain("bg-gray-200");
|
||||
});
|
||||
|
||||
it("should apply danger variant styles", () => {
|
||||
render(<Button variant="danger">Delete</Button>);
|
||||
const button = screen.getByRole("button");
|
||||
expect(button.className).toContain("bg-red-600");
|
||||
});
|
||||
});
|
||||
|
||||
it("should apply size styles", () => {
|
||||
render(<Button size="lg">Large Button</Button>);
|
||||
const button = screen.getByRole("button");
|
||||
expect(button.className).toContain("px-6");
|
||||
describe("sizes", () => {
|
||||
it("should apply medium size by default", () => {
|
||||
render(<Button>Medium</Button>);
|
||||
const button = screen.getByRole("button");
|
||||
expect(button.className).toContain("px-4");
|
||||
});
|
||||
|
||||
it("should apply small size styles", () => {
|
||||
render(<Button size="sm">Small</Button>);
|
||||
const button = screen.getByRole("button");
|
||||
expect(button.className).toContain("px-3");
|
||||
});
|
||||
|
||||
it("should apply large size styles", () => {
|
||||
render(<Button size="lg">Large Button</Button>);
|
||||
const button = screen.getByRole("button");
|
||||
expect(button.className).toContain("px-6");
|
||||
});
|
||||
});
|
||||
|
||||
describe("onClick", () => {
|
||||
it("should call onClick handler when clicked", () => {
|
||||
const handleClick = vi.fn();
|
||||
render(<Button onClick={handleClick}>Click me</Button>);
|
||||
fireEvent.click(screen.getByRole("button"));
|
||||
expect(handleClick).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("should not call onClick when disabled", () => {
|
||||
const handleClick = vi.fn();
|
||||
render(
|
||||
<Button onClick={handleClick} disabled>
|
||||
Disabled
|
||||
</Button>
|
||||
);
|
||||
fireEvent.click(screen.getByRole("button"));
|
||||
expect(handleClick).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
it("should pass through additional props", () => {
|
||||
render(<Button disabled>Disabled</Button>);
|
||||
expect(screen.getByRole("button")).toBeDisabled();
|
||||
});
|
||||
|
||||
it("should merge custom className with default styles", () => {
|
||||
render(<Button className="custom-class">Custom</Button>);
|
||||
const button = screen.getByRole("button");
|
||||
expect(button.className).toContain("custom-class");
|
||||
expect(button.className).toContain("bg-blue-600");
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user