feat(agents): add modular engineering rules from 2026 standards (#26847)
* feat(agents): add modular engineering rules from 2026 standards Add a rules directory with individual rule files derived from the Cal.com Engineering in 2026 and Beyond blog post. Rules are organized by section (architecture, quality, data, api, performance, testing, patterns, culture) following the Vercel agent-skills structure. Includes: - _sections.md defining rule categories and impact levels - _template.md for creating new rules - 14 individual rule files covering key engineering standards - README documenting the rules structure and usage Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * feat(agents): consolidate DI and Repository+DTO docs into rules - Move di-pattern.md content to rules/patterns-di-pattern.md - Extract Repository + DTO section from knowledge-base.md into: - rules/data-repository-methods.md (method naming conventions) - rules/data-dto-boundaries.md (DTO location and naming) - Update knowledge-base.md to reference the new rule files - Delete old di-pattern.md file Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * chore(agents): remove stub reference sections from knowledge-base.md The rules directory is self-contained with its own README, so these redirect sections are unnecessary clutter. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * refactor(agents): combine DI pattern rules into single file Merged patterns-di-pattern.md into patterns-dependency-injection.md to eliminate overlap and create one comprehensive DI guide. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
Keith Williams
co-authored by
Devin AI
Devin AI
parent
9f231d98a4
commit
d4ea931bf8
@@ -4,6 +4,7 @@ This directory contains comprehensive documentation for AI agents working on the
|
||||
|
||||
## Quick Navigation
|
||||
|
||||
- **[Rules](rules/)** - Modular engineering rules derived from our 2026 standards
|
||||
- **[Commands](commands.md)** - Build, test, and development commands
|
||||
- **[Knowledge Base](knowledge-base.md)** - Knowledge base & best practices
|
||||
- **[Architecture Overview](#architecture-overview)** - System structure and patterns
|
||||
|
||||
@@ -377,291 +377,6 @@ import { Button } from "@calcom/ui/components/button";
|
||||
- **Tests**: Same as source file + `.test.ts` or `.spec.ts`
|
||||
- **Avoid**: Dot-suffixes like `.service.ts`, `.repository.ts` (except for tests, types, specs)
|
||||
|
||||
## Repository + DTO Pattern and Method Conventions
|
||||
|
||||
We use a Repository + DTO pattern to isolate Prisma and the database from business logic. Repositories are the only layer that talks to Prisma and database models. All services, tRPC handlers, API controllers, workflows, and UI should depend on DTOs or domain types, not Prisma types.
|
||||
|
||||
DTOs (Data Transfer Objects) are simple TypeScript types or interfaces that represent our domain data in an ORM-agnostic way and are returned from repositories.
|
||||
|
||||
### Why this pattern?
|
||||
|
||||
- **Prevent type leaks**: Prisma type changes stay inside repositories instead of breaking services, handlers, and UI across dozens of files.
|
||||
- **Enable safe refactors**: Business logic works against stable DTOs, so you can change storage details without touching higher layers.
|
||||
- **Keep ORM swappable**: If we ever change ORM, only repository implementations change; services and handlers stay the same.
|
||||
|
||||
### What not to do: tight Prisma coupling in business logic
|
||||
|
||||
Do not import Prisma types in business logic. Prisma types belong only in the data access layer.
|
||||
|
||||
```typescript
|
||||
// ❌ Bad – business logic depends on Prisma
|
||||
import type { Webhook } from "@calcom/prisma/client";
|
||||
export function sendPayload(webhook: Webhook) { /* ... */ }
|
||||
|
||||
// ✅ Good – business logic depends on DTO + repository
|
||||
export interface WebhookDTO { id: string; url: string; }
|
||||
export interface IWebhookRepository { findById(id: string): Promise<WebhookDTO | null>; }
|
||||
export async function sendPayload(repo: IWebhookRepository, id: string) {
|
||||
const webhook = await repo.findById(id);
|
||||
// ...
|
||||
}
|
||||
```
|
||||
|
||||
### DTOs in repositories
|
||||
|
||||
Repositories should expose DTOs from their public methods and keep Prisma types internal. Inside the repository, use `Prisma.*GetPayload` and map rows to DTOs. The public methods should only return DTOs, never Prisma models.
|
||||
|
||||
```typescript
|
||||
// DTO – ORM-agnostic type exported from the repository module
|
||||
export interface BookingDTO {
|
||||
id: string;
|
||||
userId: number;
|
||||
startTime: Date;
|
||||
status: "PENDING" | "CONFIRMED" | "CANCELLED";
|
||||
}
|
||||
```
|
||||
|
||||
### DTO Location and Naming
|
||||
|
||||
**Location**: All DTOs go in `packages/lib/dto/`
|
||||
|
||||
**Naming conventions**:
|
||||
- Base entity: `{Entity}Dto` (e.g., `BookingDto`)
|
||||
- With relations: `{Entity}With{Relations}Dto` (e.g., `BookingWithAttendeesDto`)
|
||||
- For specific projections: `{Entity}For{Purpose}Dto` (e.g., `BookingForConfirmationDto`)
|
||||
- Avoid: `{Entity}Dto2`, `{Entity}DtoForHandler`, or other use-case-specific names
|
||||
|
||||
**Enum/union pattern** – use string literal unions to stay ORM-agnostic:
|
||||
|
||||
```typescript
|
||||
// ✅ Good - ORM-agnostic string literal union
|
||||
export type BookingStatusDto = "CANCELLED" | "ACCEPTED" | "REJECTED" | "PENDING";
|
||||
|
||||
// ❌ Bad - importing Prisma enum
|
||||
import { BookingStatus } from "@calcom/prisma/client";
|
||||
```
|
||||
|
||||
**Type safety** – never use `as any` in DTO mapping functions. If types don't align, fix the mapping explicitly.
|
||||
|
||||
### Prisma boundaries
|
||||
|
||||
- **Allowed**: `packages/prisma`, repository implementations (`packages/features/**/repositories/*Repository.ts`), and low-level data access infrastructure.
|
||||
- **Not allowed**: `packages/features/**` business logic (non-repository), `packages/trpc/**` handlers, `apps/web/**`, `apps/api/v2/**` services/controllers, and workflow/webhook/service layers.
|
||||
|
||||
### Method Naming Rules
|
||||
|
||||
**1. Don't include the repository's entity name in method names**
|
||||
|
||||
Method names should be concise and avoid redundancy since the repository class name already indicates the entity type.
|
||||
|
||||
```typescript
|
||||
// ✅ Good - Concise method names
|
||||
class BookingRepository {
|
||||
findById(id: string) { ... }
|
||||
findByUserId(userId: string) { ... }
|
||||
create(data: BookingCreateInput) { ... }
|
||||
delete(id: string) { ... }
|
||||
}
|
||||
|
||||
// ❌ Bad - Redundant entity name in methods
|
||||
class BookingRepository {
|
||||
findBookingById(id: string) { ... }
|
||||
findBookingByUserId(userId: string) { ... }
|
||||
createBooking(data: BookingCreateInput) { ... }
|
||||
deleteBooking(id: string) { ... }
|
||||
}
|
||||
```
|
||||
|
||||
**2. Use `include` or similar keywords for methods that fetch relational data**
|
||||
|
||||
When a method retrieves additional related entities, make this explicit in the method name using keywords like `include`, `with`, or `andRelations`.
|
||||
|
||||
```typescript
|
||||
// ✅ Good - Clear indication of included relations
|
||||
class EventTypeRepository {
|
||||
findById(id: string) {
|
||||
return prisma.eventType.findUnique({
|
||||
where: { id },
|
||||
});
|
||||
}
|
||||
|
||||
findByIdIncludeHosts(id: string) {
|
||||
return prisma.eventType.findUnique({
|
||||
where: { id },
|
||||
include: {
|
||||
hosts: true,
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
findByIdIncludeHostsAndSchedule(id: string) {
|
||||
return prisma.eventType.findUnique({
|
||||
where: { id },
|
||||
include: {
|
||||
hosts: true,
|
||||
schedule: true,
|
||||
},
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
// ❌ Bad - Unclear what data is included
|
||||
class EventTypeRepository {
|
||||
findById(id: string) {
|
||||
return prisma.eventType.findUnique({
|
||||
where: { id },
|
||||
include: {
|
||||
hosts: true,
|
||||
schedule: true,
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
findByIdForReporting(id: string) {
|
||||
return prisma.eventType.findUnique({
|
||||
where: { id },
|
||||
include: {
|
||||
hosts: true,
|
||||
},
|
||||
});
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**3. Keep methods generic and reusable - avoid use-case-specific names**
|
||||
|
||||
Repository methods should be general-purpose and describe what data they return, not how or where it's used. This promotes code reuse across different features.
|
||||
|
||||
```typescript
|
||||
// ✅ Good - Generic, reusable methods
|
||||
class BookingRepository {
|
||||
findByUserIdIncludeAttendees(userId: string) {
|
||||
return prisma.booking.findMany({
|
||||
where: { userId },
|
||||
include: {
|
||||
attendees: true,
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
findByDateRangeIncludeEventType(startDate: Date, endDate: Date) {
|
||||
return prisma.booking.findMany({
|
||||
where: {
|
||||
startTime: { gte: startDate },
|
||||
endTime: { lte: endDate },
|
||||
},
|
||||
include: {
|
||||
eventType: true,
|
||||
},
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
// ❌ Bad - Use-case-specific method names
|
||||
class BookingRepository {
|
||||
findBookingsForReporting(userId: string) {
|
||||
return prisma.booking.findMany({
|
||||
where: { userId },
|
||||
include: {
|
||||
attendees: true,
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
findBookingsForDashboard(startDate: Date, endDate: Date) {
|
||||
return prisma.booking.findMany({
|
||||
where: {
|
||||
startTime: { gte: startDate },
|
||||
endTime: { lte: endDate },
|
||||
},
|
||||
include: {
|
||||
eventType: true,
|
||||
},
|
||||
});
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**4. No business logic in repositories**
|
||||
|
||||
Repositories should only handle data access. Business logic, validations, and complex transformations belong in the Service layer.
|
||||
|
||||
```typescript
|
||||
// ✅ Good - Repository only handles data access
|
||||
class BookingRepository {
|
||||
findByIdIncludeAttendees(id: string) {
|
||||
return prisma.booking.findUnique({
|
||||
where: { id },
|
||||
include: {
|
||||
attendees: true,
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
updateStatus(id: string, status: BookingStatus) {
|
||||
return prisma.booking.update({
|
||||
where: { id },
|
||||
data: { status },
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
class BookingService {
|
||||
async confirmBooking(bookingId: string) {
|
||||
const booking = await this.bookingRepository.findByIdIncludeAttendees(bookingId);
|
||||
|
||||
if (!booking) {
|
||||
throw new Error("Booking not found");
|
||||
}
|
||||
|
||||
if (booking.status !== "PENDING") {
|
||||
throw new Error("Only pending bookings can be confirmed");
|
||||
}
|
||||
|
||||
// Business logic: send confirmation emails
|
||||
await this.emailService.sendConfirmationToAttendees(booking.attendees);
|
||||
|
||||
// Update status through repository
|
||||
return this.bookingRepository.updateStatus(bookingId, "CONFIRMED");
|
||||
}
|
||||
}
|
||||
|
||||
// ❌ Bad - Business logic in repository
|
||||
class BookingRepository {
|
||||
async confirmBooking(bookingId: string) {
|
||||
const booking = await prisma.booking.findUnique({
|
||||
where: { id: bookingId },
|
||||
include: {
|
||||
attendees: true,
|
||||
},
|
||||
});
|
||||
|
||||
if (!booking) {
|
||||
throw new Error("Booking not found");
|
||||
}
|
||||
|
||||
if (booking.status !== "PENDING") {
|
||||
throw new Error("Only pending bookings can be confirmed");
|
||||
}
|
||||
|
||||
// ❌ Business logic shouldn't be here
|
||||
await sendEmailToAttendees(booking.attendees);
|
||||
|
||||
return prisma.booking.update({
|
||||
where: { id: bookingId },
|
||||
data: { status: "CONFIRMED" },
|
||||
});
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### Summary
|
||||
|
||||
- Method names should be concise: `findById` not `findBookingById`
|
||||
- Use `include`/`with` keywords when fetching relations: `findByIdIncludeHosts`
|
||||
- Keep methods generic and reusable: `findByUserIdIncludeAttendees` not `findBookingsForReporting`
|
||||
- No business logic in repositories - that belongs in Services
|
||||
|
||||
## When using Day.js
|
||||
|
||||
```typescript
|
||||
@@ -732,7 +447,3 @@ When doing logic that depends on Browser locale, use i18n.language (prefer to de
|
||||
Note that with Date, you’re dealing with System time, so it’s not suited to everywhere (such as in the Booker, where instead we’ll likely migrate to Temporal) - but in most cases the above are suitable.
|
||||
|
||||
The main reason for doing so is that Dayjs uses a useful, but highly risky plugin system, which has led us to create `@calcom/dayjs` - this is heavy however, because it pre-loads ALL plugins, including locale handling. It's a non-ideal solution to a problem that unfortunately exists due to Dayjs.
|
||||
|
||||
## Dependency Injection (DI) Pattern
|
||||
|
||||
See [di-pattern.md](./di-pattern.md) for the complete guide on using the type-safe moduleLoader pattern for dependency injection.
|
||||
|
||||
@@ -0,0 +1,79 @@
|
||||
# Cal.com Engineering Rules
|
||||
|
||||
This directory contains modular, machine-readable engineering rules derived from [Cal.com's Engineering Standards for 2026 and Beyond](https://cal.com/blog/engineering-in-2026-and-beyond).
|
||||
|
||||
## Structure
|
||||
|
||||
Rules are organized by section prefix, as defined in `_sections.md`:
|
||||
|
||||
| Prefix | Section | Impact |
|
||||
|--------|---------|--------|
|
||||
| `architecture-` | Architecture | CRITICAL |
|
||||
| `quality-` | Code Quality | CRITICAL |
|
||||
| `data-` | Data Layer | HIGH |
|
||||
| `api-` | API Design | HIGH |
|
||||
| `performance-` | Performance | HIGH |
|
||||
| `testing-` | Testing | MEDIUM-HIGH |
|
||||
| `patterns-` | Design Patterns | MEDIUM |
|
||||
| `culture-` | Team Culture | MEDIUM |
|
||||
|
||||
## Files
|
||||
|
||||
- `_sections.md` - Defines all sections, their ordering, and impact levels
|
||||
- `_template.md` - Template for creating new rules
|
||||
- `{section}-{rule-name}.md` - Individual rule files
|
||||
|
||||
## Rule Format
|
||||
|
||||
Each rule file follows a consistent format with YAML frontmatter:
|
||||
|
||||
```markdown
|
||||
---
|
||||
title: Rule Title Here
|
||||
impact: CRITICAL | HIGH | MEDIUM | LOW
|
||||
impactDescription: Optional description (e.g., "20-50% improvement")
|
||||
tags: tag1, tag2, tag3
|
||||
---
|
||||
|
||||
## Rule Title Here
|
||||
|
||||
**Impact: LEVEL (optional description)**
|
||||
|
||||
Brief explanation of the rule and why it matters.
|
||||
|
||||
**Incorrect (description):**
|
||||
\`\`\`typescript
|
||||
// Bad code example
|
||||
\`\`\`
|
||||
|
||||
**Correct (description):**
|
||||
\`\`\`typescript
|
||||
// Good code example
|
||||
\`\`\`
|
||||
|
||||
Reference: [Link](url)
|
||||
```
|
||||
|
||||
## Adding New Rules
|
||||
|
||||
1. Copy `_template.md` to a new file with the appropriate section prefix
|
||||
2. Fill in the frontmatter (title, impact, tags)
|
||||
3. Write a clear explanation of the rule
|
||||
4. Provide incorrect and correct code examples
|
||||
5. Add a reference link if applicable
|
||||
|
||||
## Usage
|
||||
|
||||
These rules are designed to be:
|
||||
- **Human-readable**: Engineers can browse and learn from them
|
||||
- **Machine-readable**: AI agents can parse and apply them
|
||||
- **Modular**: Individual rules can be updated without affecting others
|
||||
- **Versionable**: Changes are tracked in git history
|
||||
|
||||
## Core Principles
|
||||
|
||||
From the blog post, our engineering philosophy is:
|
||||
|
||||
> We are building infrastructure that must almost never fail. To achieve this, we move fast while shipping amazing quality software with no shortcuts or compromises.
|
||||
|
||||
The rules in this directory encode the specific practices that enable this philosophy.
|
||||
@@ -0,0 +1,46 @@
|
||||
# Sections
|
||||
|
||||
This file defines all sections, their ordering, impact levels, and descriptions.
|
||||
The section ID (in parentheses) is the filename prefix used to group rules.
|
||||
|
||||
---
|
||||
|
||||
## 1. Architecture (architecture)
|
||||
|
||||
**Impact:** CRITICAL
|
||||
**Description:** Vertical Slice Architecture and Domain-Driven Design patterns that form the foundation of our codebase organization.
|
||||
|
||||
## 2. Code Quality (quality)
|
||||
|
||||
**Impact:** CRITICAL
|
||||
**Description:** Standards for maintaining high-quality, maintainable code including PR reviews, testing, and accountability.
|
||||
|
||||
## 3. Data Layer (data)
|
||||
|
||||
**Impact:** HIGH
|
||||
**Description:** Repository patterns, DTOs, and technology isolation to prevent coupling and enable maintainability.
|
||||
|
||||
## 4. API Design (api)
|
||||
|
||||
**Impact:** HIGH
|
||||
**Description:** Controller patterns, API stability, and proper separation of HTTP concerns from business logic.
|
||||
|
||||
## 5. Performance (performance)
|
||||
|
||||
**Impact:** HIGH
|
||||
**Description:** Algorithm complexity, data structure choices, and performance patterns for enterprise scale.
|
||||
|
||||
## 6. Testing (testing)
|
||||
|
||||
**Impact:** MEDIUM-HIGH
|
||||
**Description:** Test coverage requirements and testing strategies for maintaining code quality.
|
||||
|
||||
## 7. Design Patterns (patterns)
|
||||
|
||||
**Impact:** MEDIUM
|
||||
**Description:** Factory patterns, dependency injection, and other design patterns for clean, maintainable code.
|
||||
|
||||
## 8. Team Culture (culture)
|
||||
|
||||
**Impact:** MEDIUM
|
||||
**Description:** Engineering culture, accountability, and collaboration standards.
|
||||
@@ -0,0 +1,28 @@
|
||||
---
|
||||
title: Rule Title Here
|
||||
impact: MEDIUM
|
||||
impactDescription: Optional description of impact (e.g., "20-50% improvement")
|
||||
tags: tag1, tag2
|
||||
---
|
||||
|
||||
## Rule Title Here
|
||||
|
||||
**Impact: MEDIUM (optional impact description)**
|
||||
|
||||
Brief explanation of the rule and why it matters. This should be clear and concise, explaining the implications.
|
||||
|
||||
**Incorrect (description of what's wrong):**
|
||||
|
||||
```typescript
|
||||
// Bad code example here
|
||||
const bad = example()
|
||||
```
|
||||
|
||||
**Correct (description of what's right):**
|
||||
|
||||
```typescript
|
||||
// Good code example here
|
||||
const good = example()
|
||||
```
|
||||
|
||||
Reference: [Link to documentation or resource](https://example.com)
|
||||
@@ -0,0 +1,60 @@
|
||||
---
|
||||
title: Never Introduce Breaking API Changes
|
||||
impact: CRITICAL
|
||||
impactDescription: Maintains developer trust and prevents integration nightmares
|
||||
tags: api, stability, versioning, backwards-compatibility
|
||||
---
|
||||
|
||||
## Never Introduce Breaking API Changes
|
||||
|
||||
**Impact: CRITICAL**
|
||||
|
||||
Once an API endpoint is public, it must remain stable. Breaking changes destroy developer trust and create integration nightmares for our users.
|
||||
|
||||
**Strategies for avoiding breaking changes:**
|
||||
- Always add new fields as optional
|
||||
- Use API versioning when you must change existing behavior
|
||||
- Deprecate old endpoints gracefully with clear migration paths
|
||||
- Maintain backward compatibility for at least two major versions
|
||||
|
||||
**Incorrect (breaking change):**
|
||||
|
||||
```typescript
|
||||
// v1 - Original response
|
||||
interface BookingResponse {
|
||||
id: number;
|
||||
startTime: string; // ISO string
|
||||
}
|
||||
|
||||
// v1 - Breaking change: renamed field
|
||||
interface BookingResponse {
|
||||
id: number;
|
||||
start: string; // Renamed from startTime - BREAKS CLIENTS
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (non-breaking evolution):**
|
||||
|
||||
```typescript
|
||||
// v1 - Original response
|
||||
interface BookingResponse {
|
||||
id: number;
|
||||
startTime: string;
|
||||
}
|
||||
|
||||
// v1 - Non-breaking: add new field, keep old one
|
||||
interface BookingResponse {
|
||||
id: number;
|
||||
startTime: string; // Keep for backwards compatibility
|
||||
start: string; // New preferred field
|
||||
}
|
||||
```
|
||||
|
||||
**When you must make breaking changes:**
|
||||
- Create a new API version using date-specific versioning in API v2
|
||||
- Run both versions simultaneously during transition
|
||||
- Provide automated migration tools when possible
|
||||
- Give users ample time to migrate (minimum 6 months for public APIs)
|
||||
- Document exactly what changed and why
|
||||
|
||||
Reference: [Cal.com Engineering Blog](https://cal.com/blog/engineering-in-2026-and-beyond)
|
||||
@@ -0,0 +1,73 @@
|
||||
---
|
||||
title: Keep Controllers Thin - HTTP Concerns Only
|
||||
impact: HIGH
|
||||
impactDescription: Enables technology-agnostic business logic
|
||||
tags: api, controllers, http, separation-of-concerns
|
||||
---
|
||||
|
||||
## Keep Controllers Thin - HTTP Concerns Only
|
||||
|
||||
**Impact: HIGH**
|
||||
|
||||
Controllers are thin layers that handle only HTTP concerns. They take requests, process them, and map data to DTOs that are passed to core application logic. No application or core logic should be seen in API routes or tRPC handlers.
|
||||
|
||||
**Controller responsibilities (and ONLY these):**
|
||||
- Receive and validate incoming requests
|
||||
- Extract data from request parameters, body, headers
|
||||
- Transform request data into DTOs
|
||||
- Call appropriate application services with those DTOs
|
||||
- Transform application service responses into response DTOs
|
||||
- Return HTTP responses with proper status codes
|
||||
|
||||
**Controllers should NOT:**
|
||||
- Contain business logic or domain rules
|
||||
- Directly access databases or external services
|
||||
- Perform complex data transformations or calculations
|
||||
- Make decisions about what the application should do
|
||||
- Know about implementation details of the domain
|
||||
|
||||
**Incorrect (business logic in controller):**
|
||||
|
||||
```typescript
|
||||
export async function POST(request: Request) {
|
||||
const body = await request.json();
|
||||
|
||||
// Business logic in controller - BAD
|
||||
const user = await prisma.user.findFirst({ where: { id: body.userId } });
|
||||
if (!user.canBook) {
|
||||
return Response.json({ error: "Cannot book" }, { status: 403 });
|
||||
}
|
||||
|
||||
const booking = await prisma.booking.create({
|
||||
data: {
|
||||
title: body.title,
|
||||
startTime: new Date(body.startTime),
|
||||
// Complex logic here...
|
||||
}
|
||||
});
|
||||
|
||||
return Response.json(booking);
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (thin controller):**
|
||||
|
||||
```typescript
|
||||
export async function POST(request: Request) {
|
||||
const body = await request.json();
|
||||
|
||||
// Validate input
|
||||
const input = CreateBookingSchema.parse(body);
|
||||
|
||||
// Delegate to service
|
||||
const result = await bookingService.createBooking(input);
|
||||
|
||||
// Transform and return
|
||||
return Response.json(BookingResponseSchema.parse(result));
|
||||
}
|
||||
```
|
||||
|
||||
**The principle:**
|
||||
We must detach HTTP technology from our application. The way we transfer data between client and server (whether REST, tRPC, etc.) should not influence how our core application works. HTTP is a delivery mechanism, not an architectural driver.
|
||||
|
||||
Reference: [Cal.com Engineering Blog](https://cal.com/blog/engineering-in-2026-and-beyond)
|
||||
@@ -0,0 +1,42 @@
|
||||
---
|
||||
title: Enforce Feature Boundaries Through Public APIs
|
||||
impact: CRITICAL
|
||||
impactDescription: Prevents architectural erosion and maintains loose coupling
|
||||
tags: architecture, boundaries, imports, coupling
|
||||
---
|
||||
|
||||
## Enforce Feature Boundaries Through Public APIs
|
||||
|
||||
**Impact: CRITICAL**
|
||||
|
||||
Features communicate through well-defined interfaces. If bookings needs availability data, it imports from `@calcom/features/availability` through exported interfaces, not by reaching into internal implementation details.
|
||||
|
||||
**Incorrect (reaching into internals):**
|
||||
|
||||
```typescript
|
||||
// Bad - Importing internal implementation details
|
||||
import { calculateSlots } from "@calcom/features/availability/services/internal/slotCalculator";
|
||||
import { AvailabilityCache } from "@calcom/features/availability/lib/cache";
|
||||
```
|
||||
|
||||
**Correct (using public API):**
|
||||
|
||||
```typescript
|
||||
// Good - Import through the feature's public API
|
||||
import { getAvailability } from "@calcom/features/availability";
|
||||
import type { AvailabilityResult } from "@calcom/features/availability";
|
||||
```
|
||||
|
||||
**Shared code placement:**
|
||||
- Domain-agnostic utilities and cross-cutting concerns (auth, logging): `packages/lib`
|
||||
- Shared UI primitives: `packages/ui`
|
||||
|
||||
**Enforcement:**
|
||||
Domain boundaries are enforced automatically through linting. If `packages/features/bookings` tries to import from `packages/features/availability/services/internal`, the linter will block it. All cross-feature dependencies must go through the feature's public API.
|
||||
|
||||
**Benefits:**
|
||||
- Discoverability: Looking for booking logic? It's all in `packages/features/bookings`
|
||||
- Easier testing: Test the entire feature as a unit with all pieces in one place
|
||||
- Clearer dependencies: When you see `import { getAvailability } from '@calcom/features/availability'`, you know exactly which feature you're depending on
|
||||
|
||||
Reference: [Cal.com Engineering Blog](https://cal.com/blog/engineering-in-2026-and-beyond)
|
||||
@@ -0,0 +1,61 @@
|
||||
---
|
||||
title: Organize Code by Domain Using Vertical Slices
|
||||
impact: CRITICAL
|
||||
impactDescription: Dramatically improves discoverability and reduces cross-team conflicts
|
||||
tags: architecture, vertical-slices, ddd, organization
|
||||
---
|
||||
|
||||
## Organize Code by Domain Using Vertical Slices
|
||||
|
||||
**Impact: CRITICAL**
|
||||
|
||||
Our codebase is organized by domain, not by technical layer. The `packages/features` directory is the heart of this architectural approach. Each folder inside represents a complete vertical slice of the application, driven by the domain it touches.
|
||||
|
||||
**Incorrect (traditional layered architecture):**
|
||||
|
||||
```
|
||||
src/
|
||||
controllers/
|
||||
bookingController.ts
|
||||
availabilityController.ts
|
||||
services/
|
||||
bookingService.ts
|
||||
availabilityService.ts
|
||||
repositories/
|
||||
bookingRepository.ts
|
||||
availabilityRepository.ts
|
||||
```
|
||||
|
||||
This creates problems: changes to one feature require touching files scattered across multiple directories, it's hard to understand what a feature does because its code is fragmented, and teams step on each other's toes.
|
||||
|
||||
**Correct (vertical slice architecture):**
|
||||
|
||||
```
|
||||
packages/features/
|
||||
bookings/
|
||||
services/
|
||||
repositories/
|
||||
components/
|
||||
tests/
|
||||
availability/
|
||||
services/
|
||||
repositories/
|
||||
components/
|
||||
tests/
|
||||
```
|
||||
|
||||
Each feature folder is a self-contained vertical slice that includes:
|
||||
- Domain logic: Core business rules and entities specific to that feature
|
||||
- Application services: Use case orchestration for that domain
|
||||
- Repositories: Data access specific to that feature's needs
|
||||
- DTOs: Data transfer objects for crossing boundaries
|
||||
- UI components: Frontend components related to this feature
|
||||
- Tests: Unit, integration, and e2e tests for this feature
|
||||
|
||||
**Benefits:**
|
||||
- Everything related to a feature lives in one directory
|
||||
- You can understand the entire feature by exploring one directory
|
||||
- Teams can work on different features without conflicts
|
||||
- Features are loosely coupled and can evolve independently
|
||||
|
||||
Reference: [Cal.com Engineering Blog](https://cal.com/blog/engineering-in-2026-and-beyond)
|
||||
@@ -0,0 +1,45 @@
|
||||
---
|
||||
title: Hold Each Other Accountable for Quality
|
||||
impact: MEDIUM
|
||||
impactDescription: Builds collective ownership and prevents technical debt
|
||||
tags: culture, accountability, quality, teamwork
|
||||
---
|
||||
|
||||
## Hold Each Other Accountable for Quality
|
||||
|
||||
**Impact: MEDIUM**
|
||||
|
||||
We hold each other accountable for quality. Cutting corners might feel faster in the moment, but it creates problems that slow everyone down later. When you see a teammate about to merge a PR with obvious issues, speak up.
|
||||
|
||||
**This isn't about being difficult or slowing people down.** It's about collective ownership of our codebase and our reputation. Every shortcut one person takes becomes everyone's problem. Every corner cut today means more debugging sessions, more hotfixes, and more frustrated customers tomorrow.
|
||||
|
||||
**Make it normal to challenge poor decisions, respectfully:**
|
||||
|
||||
```
|
||||
// When someone says:
|
||||
"Let's just hard-code this for now"
|
||||
|
||||
// The expected response:
|
||||
"What would it take to do it the proper way the first time?"
|
||||
```
|
||||
|
||||
```
|
||||
// When someone wants to commit untested code:
|
||||
"Can we add tests for this before merging? I can help if needed."
|
||||
```
|
||||
|
||||
```
|
||||
// When someone suggests copy-paste instead of abstraction:
|
||||
"This looks like it could be a shared utility. Should we extract it?"
|
||||
```
|
||||
|
||||
**We're building something that needs to almost never fail.** That level of reliability doesn't happen by accident. It happens when every engineer feels responsible for quality - not just their own code but the entire system. We succeed as a team or we fail as a team.
|
||||
|
||||
**Key behaviors:**
|
||||
- Push back when you see shortcuts being taken
|
||||
- Offer to help when suggesting improvements
|
||||
- Accept feedback gracefully when others challenge your decisions
|
||||
- Focus on the code, not the person
|
||||
- Remember that quality standards protect everyone
|
||||
|
||||
Reference: [Cal.com Engineering Blog](https://cal.com/blog/engineering-in-2026-and-beyond)
|
||||
@@ -0,0 +1,75 @@
|
||||
---
|
||||
title: Leverage AI for Boilerplate and Testing
|
||||
impact: MEDIUM
|
||||
impactDescription: Accelerates development while maintaining quality
|
||||
tags: culture, ai, automation, testing
|
||||
---
|
||||
|
||||
## Leverage AI for Boilerplate and Testing
|
||||
|
||||
**Impact: MEDIUM**
|
||||
|
||||
Generate 80% of boilerplate and non-critical code using AI, allowing us to focus solely on complex business logic and critical architectures.
|
||||
|
||||
**Where AI excels:**
|
||||
- Generating boilerplate code (DTOs, basic CRUD operations)
|
||||
- Building comprehensive test suites
|
||||
- Creating documentation
|
||||
- Repetitive refactoring tasks
|
||||
- Code review assistance
|
||||
|
||||
**Where humans must focus:**
|
||||
- Complex business logic
|
||||
- Critical architectural decisions
|
||||
- Security-sensitive code
|
||||
- Performance-critical algorithms
|
||||
- Domain-specific edge cases
|
||||
|
||||
**Example - AI-assisted test generation:**
|
||||
|
||||
```typescript
|
||||
// Human writes the function
|
||||
export function calculateOverlap(slot: TimeSlot, busy: BusyTime): boolean {
|
||||
return slot.start < busy.end && slot.end > busy.start;
|
||||
}
|
||||
|
||||
// AI generates comprehensive tests
|
||||
describe("calculateOverlap", () => {
|
||||
it("returns true when slot starts during busy period", () => {
|
||||
// AI-generated test case
|
||||
});
|
||||
|
||||
it("returns true when slot ends during busy period", () => {
|
||||
// AI-generated test case
|
||||
});
|
||||
|
||||
it("returns false when slot is completely before busy period", () => {
|
||||
// AI-generated test case
|
||||
});
|
||||
|
||||
it("returns false when slot is completely after busy period", () => {
|
||||
// AI-generated test case
|
||||
});
|
||||
|
||||
it("returns true when slot completely contains busy period", () => {
|
||||
// AI-generated test case
|
||||
});
|
||||
|
||||
it("returns true when busy period completely contains slot", () => {
|
||||
// AI-generated test case
|
||||
});
|
||||
|
||||
// AI identifies edge cases humans might miss
|
||||
it("handles exact boundary matches correctly", () => {
|
||||
// AI-generated edge case
|
||||
});
|
||||
});
|
||||
```
|
||||
|
||||
**Our CI is the final boss:**
|
||||
- Everything in our standards document is checked before code is merged in PRs
|
||||
- No surprises make it into main
|
||||
- Checks are fast and useful
|
||||
- AI helps ensure comprehensive coverage
|
||||
|
||||
Reference: [Cal.com Engineering Blog](https://cal.com/blog/engineering-in-2026-and-beyond)
|
||||
@@ -0,0 +1,96 @@
|
||||
---
|
||||
title: Use DTOs at Every Architectural Boundary
|
||||
impact: CRITICAL
|
||||
impactDescription: Prevents technology coupling and security risks
|
||||
tags: data, dto, boundaries, security, types
|
||||
---
|
||||
|
||||
## Use DTOs at Every Architectural Boundary
|
||||
|
||||
**Impact: CRITICAL**
|
||||
|
||||
Database types should not leak to the frontend. This has become a popular shortcut in our tech stack, but it's a code smell that creates multiple problems.
|
||||
|
||||
**Problems with leaking database types:**
|
||||
- Technology coupling (Prisma types end up in React components)
|
||||
- Security risks (accidental leakage of sensitive fields)
|
||||
- Fragile contracts between server and client
|
||||
- Inability to evolve the database schema independently
|
||||
|
||||
**Incorrect (database types leaking):**
|
||||
|
||||
```typescript
|
||||
// API route returning Prisma types directly
|
||||
import type { User } from "@prisma/client";
|
||||
|
||||
export async function GET(): Promise<User> {
|
||||
const user = await prisma.user.findFirst();
|
||||
return user; // Leaks all database fields including sensitive ones
|
||||
}
|
||||
|
||||
// Frontend using Prisma types
|
||||
import type { User } from "@prisma/client";
|
||||
|
||||
function UserProfile({ user }: { user: User }) {
|
||||
// Component now coupled to database schema
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (explicit DTOs):**
|
||||
|
||||
```typescript
|
||||
// Define explicit DTOs
|
||||
interface UserDTO {
|
||||
id: number;
|
||||
name: string;
|
||||
email: string;
|
||||
// Only fields needed by the client
|
||||
}
|
||||
|
||||
// API route transforms to DTO
|
||||
export async function GET(): Promise<UserDTO> {
|
||||
const user = await userRepository.findById(id);
|
||||
return UserResponseSchema.parse(user); // Validate with Zod
|
||||
}
|
||||
|
||||
// Frontend uses DTO
|
||||
function UserProfile({ user }: { user: UserDTO }) {
|
||||
// Component decoupled from database
|
||||
}
|
||||
```
|
||||
|
||||
**The standard:**
|
||||
1. **Data layer → Application layer → API**: Transform database models into application-layer DTOs, then transform application DTOs into API-specific DTOs
|
||||
2. **API → Application layer → Data layer**: Transform API DTOs through application layer and into data-specific DTOs
|
||||
3. All DTO conversions through Zod to ensure all data is validated before sending to user
|
||||
|
||||
### DTO Location and Naming
|
||||
|
||||
**Location**: All DTOs go in `packages/lib/dto/`
|
||||
|
||||
**Naming conventions**:
|
||||
- Base entity: `{Entity}Dto` (e.g., `BookingDto`)
|
||||
- With relations: `{Entity}With{Relations}Dto` (e.g., `BookingWithAttendeesDto`)
|
||||
- For specific projections: `{Entity}For{Purpose}Dto` (e.g., `BookingForConfirmationDto`)
|
||||
- Avoid: `{Entity}Dto2`, `{Entity}DtoForHandler`, or other use-case-specific names
|
||||
|
||||
**Enum/union pattern** - use string literal unions to stay ORM-agnostic:
|
||||
|
||||
```typescript
|
||||
// Good - ORM-agnostic string literal union
|
||||
export type BookingStatusDto = "CANCELLED" | "ACCEPTED" | "REJECTED" | "PENDING";
|
||||
|
||||
// Bad - importing Prisma enum
|
||||
import { BookingStatus } from "@calcom/prisma/client";
|
||||
```
|
||||
|
||||
**Type safety** - never use `as any` in DTO mapping functions. If types don't align, fix the mapping explicitly.
|
||||
|
||||
### Prisma Boundaries
|
||||
|
||||
- **Allowed**: `packages/prisma`, repository implementations (`packages/features/**/repositories/*Repository.ts`), and low-level data access infrastructure.
|
||||
- **Not allowed**: `packages/features/**` business logic (non-repository), `packages/trpc/**` handlers, `apps/web/**`, `apps/api/v2/**` services/controllers, and workflow/webhook/service layers.
|
||||
|
||||
Yes, this requires more code. Yes, it's worth it. Explicit boundaries prevent the architectural erosion that creates long-term maintenance nightmares.
|
||||
|
||||
Reference: [Cal.com Engineering Blog](https://cal.com/blog/engineering-in-2026-and-beyond)
|
||||
@@ -0,0 +1,200 @@
|
||||
---
|
||||
title: Repository Method Naming Conventions
|
||||
impact: HIGH
|
||||
impactDescription: Improves code discoverability and reusability
|
||||
tags: data, repository, naming, conventions, methods
|
||||
---
|
||||
|
||||
## Repository Method Naming Conventions
|
||||
|
||||
**Impact: HIGH**
|
||||
|
||||
Repository methods should follow consistent naming conventions to improve discoverability and promote code reuse across different features.
|
||||
|
||||
### Rule 1: Don't include the repository's entity name in method names
|
||||
|
||||
Method names should be concise and avoid redundancy since the repository class name already indicates the entity type.
|
||||
|
||||
```typescript
|
||||
// Good - Concise method names
|
||||
class BookingRepository {
|
||||
findById(id: string) { ... }
|
||||
findByUserId(userId: string) { ... }
|
||||
create(data: BookingCreateInput) { ... }
|
||||
delete(id: string) { ... }
|
||||
}
|
||||
|
||||
// Bad - Redundant entity name in methods
|
||||
class BookingRepository {
|
||||
findBookingById(id: string) { ... }
|
||||
findBookingByUserId(userId: string) { ... }
|
||||
createBooking(data: BookingCreateInput) { ... }
|
||||
deleteBooking(id: string) { ... }
|
||||
}
|
||||
```
|
||||
|
||||
### Rule 2: Use `include` or similar keywords for methods that fetch relational data
|
||||
|
||||
When a method retrieves additional related entities, make this explicit in the method name using keywords like `include`, `with`, or `andRelations`.
|
||||
|
||||
```typescript
|
||||
// Good - Clear indication of included relations
|
||||
class EventTypeRepository {
|
||||
findById(id: string) {
|
||||
return prisma.eventType.findUnique({
|
||||
where: { id },
|
||||
});
|
||||
}
|
||||
|
||||
findByIdIncludeHosts(id: string) {
|
||||
return prisma.eventType.findUnique({
|
||||
where: { id },
|
||||
include: { hosts: true },
|
||||
});
|
||||
}
|
||||
|
||||
findByIdIncludeHostsAndSchedule(id: string) {
|
||||
return prisma.eventType.findUnique({
|
||||
where: { id },
|
||||
include: { hosts: true, schedule: true },
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
// Bad - Unclear what data is included
|
||||
class EventTypeRepository {
|
||||
findById(id: string) {
|
||||
return prisma.eventType.findUnique({
|
||||
where: { id },
|
||||
include: { hosts: true, schedule: true },
|
||||
});
|
||||
}
|
||||
|
||||
findByIdForReporting(id: string) {
|
||||
return prisma.eventType.findUnique({
|
||||
where: { id },
|
||||
include: { hosts: true },
|
||||
});
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### Rule 3: Keep methods generic and reusable - avoid use-case-specific names
|
||||
|
||||
Repository methods should be general-purpose and describe what data they return, not how or where it's used. This promotes code reuse across different features.
|
||||
|
||||
```typescript
|
||||
// Good - Generic, reusable methods
|
||||
class BookingRepository {
|
||||
findByUserIdIncludeAttendees(userId: string) {
|
||||
return prisma.booking.findMany({
|
||||
where: { userId },
|
||||
include: { attendees: true },
|
||||
});
|
||||
}
|
||||
|
||||
findByDateRangeIncludeEventType(startDate: Date, endDate: Date) {
|
||||
return prisma.booking.findMany({
|
||||
where: {
|
||||
startTime: { gte: startDate },
|
||||
endTime: { lte: endDate },
|
||||
},
|
||||
include: { eventType: true },
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
// Bad - Use-case-specific method names
|
||||
class BookingRepository {
|
||||
findBookingsForReporting(userId: string) {
|
||||
return prisma.booking.findMany({
|
||||
where: { userId },
|
||||
include: { attendees: true },
|
||||
});
|
||||
}
|
||||
|
||||
findBookingsForDashboard(startDate: Date, endDate: Date) {
|
||||
return prisma.booking.findMany({
|
||||
where: {
|
||||
startTime: { gte: startDate },
|
||||
endTime: { lte: endDate },
|
||||
},
|
||||
include: { eventType: true },
|
||||
});
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### Rule 4: No business logic in repositories
|
||||
|
||||
Repositories should only handle data access. Business logic, validations, and complex transformations belong in the Service layer.
|
||||
|
||||
```typescript
|
||||
// Good - Repository only handles data access
|
||||
class BookingRepository {
|
||||
findByIdIncludeAttendees(id: string) {
|
||||
return prisma.booking.findUnique({
|
||||
where: { id },
|
||||
include: { attendees: true },
|
||||
});
|
||||
}
|
||||
|
||||
updateStatus(id: string, status: BookingStatus) {
|
||||
return prisma.booking.update({
|
||||
where: { id },
|
||||
data: { status },
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
class BookingService {
|
||||
async confirmBooking(bookingId: string) {
|
||||
const booking = await this.bookingRepository.findByIdIncludeAttendees(bookingId);
|
||||
|
||||
if (!booking) {
|
||||
throw new Error("Booking not found");
|
||||
}
|
||||
|
||||
if (booking.status !== "PENDING") {
|
||||
throw new Error("Only pending bookings can be confirmed");
|
||||
}
|
||||
|
||||
await this.emailService.sendConfirmationToAttendees(booking.attendees);
|
||||
return this.bookingRepository.updateStatus(bookingId, "CONFIRMED");
|
||||
}
|
||||
}
|
||||
|
||||
// Bad - Business logic in repository
|
||||
class BookingRepository {
|
||||
async confirmBooking(bookingId: string) {
|
||||
const booking = await prisma.booking.findUnique({
|
||||
where: { id: bookingId },
|
||||
include: { attendees: true },
|
||||
});
|
||||
|
||||
if (!booking) {
|
||||
throw new Error("Booking not found");
|
||||
}
|
||||
|
||||
if (booking.status !== "PENDING") {
|
||||
throw new Error("Only pending bookings can be confirmed");
|
||||
}
|
||||
|
||||
await sendEmailToAttendees(booking.attendees);
|
||||
|
||||
return prisma.booking.update({
|
||||
where: { id: bookingId },
|
||||
data: { status: "CONFIRMED" },
|
||||
});
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### Summary
|
||||
|
||||
- Method names should be concise: `findById` not `findBookingById`
|
||||
- Use `include`/`with` keywords when fetching relations: `findByIdIncludeHosts`
|
||||
- Keep methods generic and reusable: `findByUserIdIncludeAttendees` not `findBookingsForReporting`
|
||||
- No business logic in repositories - that belongs in Services
|
||||
|
||||
Reference: [Cal.com Engineering Blog](https://cal.com/blog/engineering-in-2026-and-beyond)
|
||||
@@ -0,0 +1,65 @@
|
||||
---
|
||||
title: Isolate Technology Choices Behind Repositories
|
||||
impact: CRITICAL
|
||||
impactDescription: Enables technology changes without codebase-wide refactors
|
||||
tags: data, repository, prisma, orm, isolation
|
||||
---
|
||||
|
||||
## Isolate Technology Choices Behind Repositories
|
||||
|
||||
**Impact: CRITICAL**
|
||||
|
||||
Technology choices must not seep through the application. The Prisma problem illustrates this perfectly: we currently have references to Prisma scattered across hundreds of files. This creates massive coupling and makes technology changes prohibitively expensive.
|
||||
|
||||
**Incorrect (Prisma leaking throughout codebase):**
|
||||
|
||||
```typescript
|
||||
// In a service file
|
||||
import { prisma } from "@calcom/prisma";
|
||||
|
||||
async function getBooking(id: number) {
|
||||
// Direct Prisma usage in service
|
||||
return prisma.booking.findFirst({
|
||||
where: { id },
|
||||
include: { user: true }
|
||||
});
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (Repository abstraction):**
|
||||
|
||||
```typescript
|
||||
// In repository file
|
||||
import { prisma } from "@calcom/prisma";
|
||||
|
||||
export class BookingRepository {
|
||||
async findById(id: number): Promise<BookingDTO | null> {
|
||||
const booking = await prisma.booking.findFirst({
|
||||
where: { id },
|
||||
select: { id: true, title: true, userId: true }
|
||||
});
|
||||
return booking ? this.toDTO(booking) : null;
|
||||
}
|
||||
}
|
||||
|
||||
// In service file - no Prisma knowledge
|
||||
import { BookingRepository } from "./repositories/BookingRepository";
|
||||
|
||||
async function getBooking(id: number) {
|
||||
return this.bookingRepository.findById(id);
|
||||
}
|
||||
```
|
||||
|
||||
**The standard:**
|
||||
- All database access must go through Repository classes
|
||||
- Repositories are the only code that knows about Prisma (or any other ORM)
|
||||
- No business logic should be in repositories
|
||||
- Repositories are injected via Dependency Injection containers
|
||||
|
||||
**Benefits:**
|
||||
If we ever switch from Prisma to Drizzle or another ORM, the only changes required are:
|
||||
- Repository implementations
|
||||
- DI container wiring for new repositories
|
||||
- Nothing else in the codebase should care or change
|
||||
|
||||
Reference: [Cal.com Engineering Blog](https://cal.com/blog/engineering-in-2026-and-beyond)
|
||||
@@ -1,12 +1,64 @@
|
||||
# Dependency Injection (DI) Pattern
|
||||
---
|
||||
title: Use Dependency Injection for Loose Coupling
|
||||
impact: HIGH
|
||||
impactDescription: Enables build-time safety, testability, and maintainability
|
||||
tags: patterns, dependency-injection, di, ioctopus, moduleloader, testing, coupling
|
||||
---
|
||||
|
||||
We use a Dependency Injection pattern powered by `@evyweb/ioctopus` to manage service and repository dependencies. This pattern ensures proper dependency management, testability, and consistent instantiation of services throughout the codebase.
|
||||
## Use Dependency Injection for Loose Coupling
|
||||
|
||||
We use the **moduleLoader pattern** which provides type-safe dependency injection. This ensures that if a service adds a new dependency, TypeScript will catch missing dependencies at build time rather than runtime.
|
||||
**Impact: HIGH**
|
||||
|
||||
## Core Concepts
|
||||
Dependency Injection enables loose coupling, facilitates testing, and isolates concerns. Dependencies should be injected via DI containers rather than instantiated directly within classes.
|
||||
|
||||
The DI system consists of four main components:
|
||||
**Incorrect (tight coupling with direct instantiation):**
|
||||
|
||||
```typescript
|
||||
class BookingService {
|
||||
private repository = new BookingRepository();
|
||||
private emailService = new EmailService();
|
||||
private calendarService = new GoogleCalendarService();
|
||||
|
||||
async createBooking(data: CreateBookingDTO) {
|
||||
const booking = await this.repository.create(data);
|
||||
await this.emailService.sendConfirmation(booking);
|
||||
await this.calendarService.createEvent(booking);
|
||||
return booking;
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (dependency injection):**
|
||||
|
||||
```typescript
|
||||
class BookingService {
|
||||
constructor(
|
||||
private readonly repository: BookingRepository,
|
||||
private readonly emailService: EmailService,
|
||||
private readonly calendarService: CalendarService,
|
||||
) {}
|
||||
|
||||
async createBooking(data: CreateBookingDTO) {
|
||||
const booking = await this.repository.create(data);
|
||||
await this.emailService.sendConfirmation(booking);
|
||||
await this.calendarService.createEvent(booking);
|
||||
return booking;
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Required patterns:**
|
||||
- **Application Services**: Orchestrate use cases, coordinate between domain services and repositories
|
||||
- **Domain Services**: Contain business logic that doesn't naturally belong to a single entity
|
||||
- **Repositories**: Abstract data access, isolate technology choices
|
||||
- **Caching Proxies**: Wrap repositories or services to add caching behavior transparently
|
||||
- **Decorators**: Add cross-cutting concerns (logging, metrics) without polluting domain logic
|
||||
|
||||
## Cal.com's Type-Safe DI with moduleLoader
|
||||
|
||||
We use `@evyweb/ioctopus` to manage service and repository dependencies. The **moduleLoader pattern** provides type-safe dependency injection, ensuring that if a service adds a new dependency, TypeScript will catch missing dependencies at build time rather than runtime.
|
||||
|
||||
### Core Concepts
|
||||
|
||||
**Tokens**: Unique symbols that identify each service or repository in the DI container. Every injectable class needs a corresponding token. Tokens should be defined in a `tokens.ts` file within the feature's `di/` directory.
|
||||
|
||||
@@ -16,9 +68,7 @@ The DI system consists of four main components:
|
||||
|
||||
**`bindModuleToClassOnToken`**: A type-safe function that binds a class to a token and declares its dependencies. TypeScript ensures the declared dependencies match what the class constructor expects.
|
||||
|
||||
## How It Works
|
||||
|
||||
Here's the complete flow for creating a DI-enabled service:
|
||||
### How It Works
|
||||
|
||||
**Step 1: Create tokens in the feature's di directory**
|
||||
|
||||
@@ -96,7 +146,6 @@ const loadModule = bindModuleToClassOnToken({
|
||||
moduleToken,
|
||||
token,
|
||||
classs: MyService,
|
||||
// For multiple dependencies, use depsMap:
|
||||
depsMap: {
|
||||
bookingRepo: bookingRepositoryModuleLoader,
|
||||
userRepo: userRepositoryModuleLoader,
|
||||
@@ -115,31 +164,13 @@ For a single dependency, use `dep` instead of `depsMap`:
|
||||
|
||||
```typescript
|
||||
// packages/features/myfeature/di/MyRepository.module.ts
|
||||
import { bindModuleToClassOnToken, createModule, type ModuleLoader } from "@calcom/features/di/di";
|
||||
import { MyRepository } from "@calcom/features/myfeature/repositories/MyRepository";
|
||||
|
||||
import { moduleLoader as prismaModuleLoader } from "@calcom/features/di/modules/Prisma";
|
||||
import { MY_FEATURE_DI_TOKENS } from "./tokens";
|
||||
|
||||
const thisModule = createModule();
|
||||
const token = MY_FEATURE_DI_TOKENS.MY_REPOSITORY;
|
||||
const moduleToken = MY_FEATURE_DI_TOKENS.MY_REPOSITORY_MODULE;
|
||||
|
||||
const loadModule = bindModuleToClassOnToken({
|
||||
module: thisModule,
|
||||
moduleToken,
|
||||
token,
|
||||
classs: MyRepository,
|
||||
// For single dependency, use dep:
|
||||
dep: prismaModuleLoader,
|
||||
});
|
||||
|
||||
export const moduleLoader: ModuleLoader = {
|
||||
token,
|
||||
loadModule,
|
||||
};
|
||||
|
||||
export type { MyRepository };
|
||||
```
|
||||
|
||||
**Step 4: Create a container that uses the moduleLoader**
|
||||
@@ -147,7 +178,6 @@ export type { MyRepository };
|
||||
```typescript
|
||||
// packages/features/myfeature/di/MyService.container.ts
|
||||
import { createContainer } from "@calcom/features/di/di";
|
||||
|
||||
import { type MyService, moduleLoader as myServiceModuleLoader } from "./MyService.module";
|
||||
|
||||
const myServiceContainer = createContainer();
|
||||
@@ -161,34 +191,16 @@ export function getMyService(): MyService {
|
||||
**Step 5: Use the service via the container's getter function**
|
||||
|
||||
```typescript
|
||||
// Anywhere in the codebase
|
||||
import { getMyService } from "@calcom/features/myfeature/di/MyService.container";
|
||||
|
||||
const myService = getMyService();
|
||||
await myService.doSomething();
|
||||
```
|
||||
|
||||
## Why moduleLoader Is Type-Safe
|
||||
|
||||
The `bindModuleToClassOnToken` function in `packages/features/di/di.ts` uses TypeScript generics to infer the required dependencies from the class constructor:
|
||||
|
||||
```typescript
|
||||
depsMap: Record<
|
||||
keyof (TClass extends new (deps: infer TDeps) => any ? TDeps : never),
|
||||
ModuleLoader
|
||||
>
|
||||
```
|
||||
|
||||
This means if `MyService` adds a new constructor dependency (e.g., `eventTypeRepo`), TypeScript will error because `depsMap` is missing that key. The error happens at build time, not runtime.
|
||||
|
||||
The `loadModule` function also automatically loads all dependencies recursively, so you don't need to manually track the dependency chain.
|
||||
|
||||
## Common Mistakes to Avoid
|
||||
|
||||
**Mistake 1: Creating a repository or service class with all static methods**
|
||||
|
||||
Static methods bypass the DI system entirely, making the code harder to test and breaking the dependency chain.
|
||||
|
||||
```typescript
|
||||
// Bad - Static methods bypass DI
|
||||
export class BookingRepository {
|
||||
@@ -197,11 +209,6 @@ export class BookingRepository {
|
||||
}
|
||||
}
|
||||
|
||||
// Usage (wrong - no DI)
|
||||
const booking = await BookingRepository.findById("123");
|
||||
```
|
||||
|
||||
```typescript
|
||||
// Good - Instance methods with constructor injection
|
||||
export class BookingRepository {
|
||||
constructor(private prismaClient: PrismaClient) {}
|
||||
@@ -210,54 +217,33 @@ export class BookingRepository {
|
||||
return this.prismaClient.booking.findUnique({ where: { id } });
|
||||
}
|
||||
}
|
||||
|
||||
// Usage (correct - via DI container)
|
||||
const bookingRepo = getBookingRepository();
|
||||
const booking = await bookingRepo.findById("123");
|
||||
```
|
||||
|
||||
**Mistake 2: Manually instantiating a class instead of using the DI container**
|
||||
|
||||
Even if you define a class with constructor injection, manually calling `new` bypasses the DI system and its benefits.
|
||||
|
||||
```typescript
|
||||
// Bad - Manual instantiation bypasses DI
|
||||
import { MyService } from "@calcom/features/myfeature/services/MyService";
|
||||
import { BookingRepository } from "@calcom/features/bookings/repositories/BookingRepository";
|
||||
import prisma from "@calcom/prisma";
|
||||
|
||||
const bookingRepo = new BookingRepository(prisma);
|
||||
const myService = new MyService({ bookingRepo });
|
||||
```
|
||||
|
||||
```typescript
|
||||
// Good - Use the DI container's getter function
|
||||
import { getMyService } from "@calcom/features/myfeature/di/MyService.container";
|
||||
|
||||
const myService = getMyService();
|
||||
```
|
||||
|
||||
**Mistake 3: Importing Prisma directly in a service instead of using repository injection**
|
||||
|
||||
Services should depend on repositories, not directly on Prisma. This maintains proper separation of concerns.
|
||||
|
||||
```typescript
|
||||
// Bad - Service imports Prisma directly
|
||||
import prisma from "@calcom/prisma";
|
||||
|
||||
export class MyService {
|
||||
async doSomething() {
|
||||
const bookings = await prisma.booking.findMany({...}); // Wrong!
|
||||
const bookings = await prisma.booking.findMany({...});
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
```typescript
|
||||
// Good - Service depends on repository via DI
|
||||
export interface IMyServiceDeps {
|
||||
bookingRepo: BookingRepository;
|
||||
}
|
||||
|
||||
export class MyService {
|
||||
constructor(private deps: IMyServiceDeps) {}
|
||||
|
||||
@@ -269,38 +255,26 @@ export class MyService {
|
||||
|
||||
**Mistake 4: Manual module loading in containers (not type-safe)**
|
||||
|
||||
Don't manually call `container.load()` for each dependency. This is not type-safe and can lead to runtime errors if you forget to load a dependency.
|
||||
|
||||
```typescript
|
||||
// Bad - Manual module loading is not type-safe
|
||||
const container = createContainer();
|
||||
container.load(DI_TOKENS.PRISMA_MODULE, prismaModule);
|
||||
container.load(DI_TOKENS.BOOKING_REPOSITORY_MODULE, bookingRepositoryModule);
|
||||
container.load(DI_TOKENS.MY_SERVICE_MODULE, myServiceModule);
|
||||
// If MyService adds a new dependency, TypeScript won't catch the missing load()
|
||||
|
||||
export function getMyService() {
|
||||
return container.get<MyService>(DI_TOKENS.MY_SERVICE);
|
||||
}
|
||||
```
|
||||
|
||||
```typescript
|
||||
// Good - Use moduleLoader for type-safe dependency loading
|
||||
import { moduleLoader as myServiceModuleLoader } from "./MyService.module";
|
||||
|
||||
const container = createContainer();
|
||||
|
||||
export function getMyService(): MyService {
|
||||
myServiceModuleLoader.loadModule(container); // Automatically loads all dependencies
|
||||
myServiceModuleLoader.loadModule(container);
|
||||
return container.get<MyService>(myServiceModuleLoader.token);
|
||||
}
|
||||
```
|
||||
|
||||
## Why Use DI?
|
||||
|
||||
- **Build-time safety**: TypeScript catches missing dependencies before runtime when using the moduleLoader pattern
|
||||
- **Testability**: Dependencies can be easily mocked in tests by providing alternative implementations
|
||||
- **Build-time safety**: TypeScript catches missing dependencies before runtime
|
||||
- **Testability**: Dependencies can be easily mocked in tests
|
||||
- **Consistency**: All instances are created the same way with proper dependencies
|
||||
- **Maintainability**: Changing a dependency only requires updating the module binding, not every usage site
|
||||
- **Maintainability**: Changing a dependency only requires updating the module binding
|
||||
- **Automatic dependency resolution**: The moduleLoader automatically loads all dependencies recursively
|
||||
- **Self-documenting**: Each module declares its own dependencies, making the dependency graph explicit
|
||||
- **Self-documenting**: Each module declares its own dependencies explicitly
|
||||
|
||||
Reference: [Cal.com Engineering Blog](https://cal.com/blog/engineering-in-2026-and-beyond)
|
||||
@@ -0,0 +1,87 @@
|
||||
---
|
||||
title: Use Factory Pattern to Push Conditionals to Entry Points
|
||||
impact: HIGH
|
||||
impactDescription: Keeps services focused and prevents complexity accumulation
|
||||
tags: patterns, factory, conditionals, single-responsibility
|
||||
---
|
||||
|
||||
## Use Factory Pattern to Push Conditionals to Entry Points
|
||||
|
||||
**Impact: HIGH**
|
||||
|
||||
If statements belong at the entry point, not scattered throughout your services. This is one of the most important architectural principles for maintaining clean, focused code that doesn't spiral into unmaintainable complexity.
|
||||
|
||||
**The problem with scattered conditionals:**
|
||||
A service is written for a clear, specific purpose. Then a new product requirement arrives, and someone adds an if statement. A few years later, that service is littered with conditional checks. The service becomes:
|
||||
- Complicated and hard to read
|
||||
- Difficult to understand and reason about
|
||||
- More susceptible to bugs
|
||||
- Violating single responsibility
|
||||
- Nearly impossible to test thoroughly
|
||||
|
||||
**Incorrect (conditionals scattered in service):**
|
||||
|
||||
```typescript
|
||||
class BillingService {
|
||||
async processPayment(entityId: number, entityType: string) {
|
||||
if (entityType === "organization") {
|
||||
// Organization-specific logic
|
||||
const org = await this.getOrganization(entityId);
|
||||
if (org.billingPlan === "enterprise") {
|
||||
// More nested conditionals...
|
||||
}
|
||||
} else if (entityType === "team") {
|
||||
// Team-specific logic
|
||||
} else if (entityType === "user") {
|
||||
// User-specific logic
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (Factory pattern with specialized services):**
|
||||
|
||||
```typescript
|
||||
// Factory makes the decision at entry point
|
||||
class BillingServiceFactory {
|
||||
static async createService(entityId: number): Promise<BillingService> {
|
||||
const entity = await determineEntityType(entityId);
|
||||
|
||||
switch (entity.type) {
|
||||
case "organization":
|
||||
return new OrganizationBillingService(entity);
|
||||
case "team":
|
||||
return new TeamBillingService(entity);
|
||||
default:
|
||||
return new UserBillingService(entity);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Each service handles ONLY its specific logic - no conditionals
|
||||
class OrganizationBillingService extends BillingService {
|
||||
async processPayment() {
|
||||
// Only organization logic here - clean and focused
|
||||
}
|
||||
}
|
||||
|
||||
class TeamBillingService extends BillingService {
|
||||
async processPayment() {
|
||||
// Only team logic here - clean and focused
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Benefits:**
|
||||
- Services stay focused with one responsibility
|
||||
- Changes are isolated to specific service implementations
|
||||
- Testing is straightforward - test each service independently
|
||||
- New requirements don't pollute existing code
|
||||
|
||||
**Guidelines:**
|
||||
- Push conditionals up to controllers, factories, or routing logic
|
||||
- Keep services pure and focused on a single responsibility
|
||||
- Prefer polymorphism over conditionals
|
||||
- Watch for if statement accumulation during code review
|
||||
|
||||
Reference: [Cal.com Engineering Blog](https://cal.com/blog/engineering-in-2026-and-beyond)
|
||||
@@ -0,0 +1,54 @@
|
||||
---
|
||||
title: Avoid O(n²) Algorithms - Design for Enterprise Scale
|
||||
impact: CRITICAL
|
||||
impactDescription: Prevents performance collapse at scale
|
||||
tags: performance, algorithms, complexity, scale
|
||||
---
|
||||
|
||||
## Avoid O(n²) Algorithms - Design for Enterprise Scale
|
||||
|
||||
**Impact: CRITICAL**
|
||||
|
||||
We build for large organizations and teams. What works fine with 10 users or 50 records can collapse under the weight of enterprise scale. Performance is not something we optimize later. It's something we build correctly from the start.
|
||||
|
||||
When building features, always ask: "How does this behave with 1,000 users? 10,000 records? 100,000 operations?"
|
||||
|
||||
**Common O(n²) patterns to avoid:**
|
||||
- Nested array iterations (`.map` inside `.map`, `.forEach` inside `.forEach`)
|
||||
- Array methods like `.some`, `.find`, or `.filter` inside loops or callbacks
|
||||
- Checking every item against every other item without optimization
|
||||
- Chained filters or nested mapping over large lists
|
||||
|
||||
**Incorrect (O(n²) - exponential slowdown):**
|
||||
|
||||
```typescript
|
||||
// Bad: O(n²) - checks every slot against every busy time
|
||||
const available = availableSlots.filter(slot => {
|
||||
return !busyTimes.some(busy => checkOverlap(slot, busy));
|
||||
});
|
||||
|
||||
// For 100 slots and 50 busy periods: 5,000 checks
|
||||
// For 500 slots and 200 busy periods: 100,000 checks (20x increase!)
|
||||
```
|
||||
|
||||
**Correct (O(n log n) - scales gracefully):**
|
||||
|
||||
```typescript
|
||||
// Good: O(n log n) - sort once, break early
|
||||
const sortedBusy = [...busyTimes].sort((a, b) => a.start - b.start);
|
||||
|
||||
const available = availableSlots.filter(slot => {
|
||||
// Binary search or early exit
|
||||
const index = binarySearch(sortedBusy, slot.start);
|
||||
return !hasOverlapAt(sortedBusy, index, slot);
|
||||
});
|
||||
```
|
||||
|
||||
**Better data structures and algorithms:**
|
||||
- **Sorting + early exit**: Sort data once, break out of loops when remaining items won't match
|
||||
- **Binary search**: Use for lookups in sorted arrays instead of linear scans
|
||||
- **Two-pointer techniques**: For merging or intersecting sorted sequences
|
||||
- **Hash maps/sets**: Use for O(1) lookups instead of `.find` or `.includes` on arrays
|
||||
- **Interval trees**: For scheduling, availability, and range queries
|
||||
|
||||
Reference: [Cal.com Engineering Blog](https://cal.com/blog/engineering-in-2026-and-beyond)
|
||||
@@ -0,0 +1,58 @@
|
||||
---
|
||||
title: Handle NP-Hard Scheduling Problems Carefully
|
||||
impact: HIGH
|
||||
impactDescription: Prevents exponential blowup in scheduling operations
|
||||
tags: performance, scheduling, algorithms, np-hard
|
||||
---
|
||||
|
||||
## Handle NP-Hard Scheduling Problems Carefully
|
||||
|
||||
**Impact: HIGH**
|
||||
|
||||
Scheduling problems are fundamentally NP-hard. This means that as the number of constraints, participants, or time slots grows, the computational complexity can explode exponentially. Most optimal scheduling algorithms have worst-case exponential time complexity, making algorithm choice absolutely critical.
|
||||
|
||||
**Real-world implications:**
|
||||
- Finding the optimal meeting time for 10 people across 3 time zones with individual availability constraints is computationally expensive
|
||||
- Adding conflict detection, buffers, and other options amplifies the problem
|
||||
- Poor algorithm choices that work fine for small teams become completely unusable for large organizations
|
||||
- What takes milliseconds for 5 users might take many seconds for organizations
|
||||
|
||||
**Strategies for managing NP-hard complexity:**
|
||||
|
||||
```typescript
|
||||
// Use approximation algorithms
|
||||
async function findMeetingTime(participants: User[], duration: number) {
|
||||
// Find "good enough" solution quickly rather than perfect solution slowly
|
||||
const approximateSlots = await findApproximateAvailability(participants, {
|
||||
maxIterations: 1000,
|
||||
timeout: 500, // ms
|
||||
});
|
||||
|
||||
return approximateSlots[0]; // Return first good-enough option
|
||||
}
|
||||
|
||||
// Implement aggressive caching
|
||||
const cachedAvailability = new LRUCache<string, Availability>({
|
||||
max: 10000,
|
||||
ttl: 1000 * 60 * 5, // 5 minutes
|
||||
});
|
||||
|
||||
// Pre-compute common scenarios during off-peak hours
|
||||
async function precomputeTeamAvailability(teamId: number) {
|
||||
// Run during low-traffic periods
|
||||
const team = await teamRepository.findById(teamId);
|
||||
const availability = await computeTeamAvailability(team);
|
||||
await cache.set(`team:${teamId}:availability`, availability);
|
||||
}
|
||||
```
|
||||
|
||||
**Key strategies:**
|
||||
- Use approximation algorithms that find "good enough" solutions quickly
|
||||
- Implement aggressive caching of computed schedules and availability
|
||||
- Pre-compute common scenarios during off-peak hours
|
||||
- Break large scheduling problems into smaller, more manageable chunks
|
||||
- Set reasonable timeout limits and fallback to simpler algorithms when needed
|
||||
|
||||
This is why performance isn't just a nice-to-have in scheduling software. It's the foundation that determines whether your system can scale to enterprise needs.
|
||||
|
||||
Reference: [Cal.com Engineering Blog](https://cal.com/blog/engineering-in-2026-and-beyond)
|
||||
@@ -0,0 +1,40 @@
|
||||
---
|
||||
title: Minimize Follow-up PRs for Small Refactors
|
||||
impact: HIGH
|
||||
impactDescription: Prevents technical debt accumulation
|
||||
tags: quality, pr, refactoring, technical-debt
|
||||
---
|
||||
|
||||
## Minimize Follow-up PRs for Small Refactors
|
||||
|
||||
**Impact: HIGH**
|
||||
|
||||
Follow-up PRs for minor improvements rarely materialize. Instead, they accumulate as technical debt that burdens us months or years later. If a small refactor can be done now, do it now.
|
||||
|
||||
**Incorrect:**
|
||||
|
||||
```typescript
|
||||
// PR comment: "I'll fix this naming in a follow-up PR"
|
||||
const d = getUserData(); // Bad variable name
|
||||
const r = processData(d); // Another bad name
|
||||
|
||||
// The follow-up PR never happens, and now we have unclear code
|
||||
```
|
||||
|
||||
**Correct:**
|
||||
|
||||
```typescript
|
||||
// Fix it in the same PR
|
||||
const userData = getUserData();
|
||||
const processedResult = processData(userData);
|
||||
```
|
||||
|
||||
**When follow-ups are acceptable:**
|
||||
- Substantial changes that genuinely warrant separate PRs
|
||||
- Exceptional, urgent cases where the current PR must ship immediately
|
||||
- Changes that require additional review from different stakeholders
|
||||
|
||||
**The principle:**
|
||||
The "slowness" of doing things right is an investment, not a cost. Code that's architected correctly from the start doesn't need massive refactors later.
|
||||
|
||||
Reference: [Cal.com Engineering Blog](https://cal.com/blog/engineering-in-2026-and-beyond)
|
||||
@@ -0,0 +1,43 @@
|
||||
---
|
||||
title: Prioritize Clarity Over Cleverness
|
||||
impact: HIGH
|
||||
impactDescription: Reduces cognitive load and improves maintainability
|
||||
tags: quality, simplicity, readability
|
||||
---
|
||||
|
||||
## Prioritize Clarity Over Cleverness
|
||||
|
||||
**Impact: HIGH**
|
||||
|
||||
The goal is code that is easy to read and understand quickly, not elegant complexity. Simple systems reduce the cognitive load for every engineer.
|
||||
|
||||
**Questions to ask yourself:**
|
||||
- Am I actually solving the problem at hand?
|
||||
- Am I thinking too much about possible future use cases?
|
||||
- Have I considered at least 1 other alternative for solving this? How does it compare?
|
||||
|
||||
**Incorrect (clever but hard to understand):**
|
||||
|
||||
```typescript
|
||||
// Clever one-liner that's hard to parse
|
||||
const result = data.reduce((a, b) => ({...a, [b.id]: (a[b.id] || []).concat(b)}), {});
|
||||
```
|
||||
|
||||
**Correct (clear and readable):**
|
||||
|
||||
```typescript
|
||||
// Clear, step-by-step approach
|
||||
const groupedById: Record<string, Item[]> = {};
|
||||
|
||||
for (const item of data) {
|
||||
if (!groupedById[item.id]) {
|
||||
groupedById[item.id] = [];
|
||||
}
|
||||
groupedById[item.id].push(item);
|
||||
}
|
||||
```
|
||||
|
||||
**Important note:**
|
||||
Simple doesn't mean lacking in features. Just because our goal is to create simple systems, this doesn't mean they should feel anemic and lacking obvious functionality.
|
||||
|
||||
Reference: [Cal.com Engineering Blog](https://cal.com/blog/engineering-in-2026-and-beyond)
|
||||
@@ -0,0 +1,40 @@
|
||||
---
|
||||
title: Address All Nits Before Merging
|
||||
impact: HIGH
|
||||
impactDescription: Prevents codebase degradation over time
|
||||
tags: quality, code-review, standards
|
||||
---
|
||||
|
||||
## Address All Nits Before Merging
|
||||
|
||||
**Impact: HIGH**
|
||||
|
||||
Don't let PRs through with a lot of nits just to avoid being "the bad person." This is precisely how codebases become sloppy over time. Code review is not about being nice. It's about maintaining the quality standards our infrastructure demands.
|
||||
|
||||
**Incorrect approach:**
|
||||
|
||||
```
|
||||
Reviewer: "This variable name could be clearer, but it's fine I guess"
|
||||
Reviewer: "We usually use early returns here, but this works"
|
||||
Reviewer: "Approved with minor suggestions"
|
||||
// PR merged with multiple small issues
|
||||
```
|
||||
|
||||
**Correct approach:**
|
||||
|
||||
```
|
||||
Reviewer: "Please rename `d` to `userData` for clarity"
|
||||
Reviewer: "Please refactor to use early returns per our standards"
|
||||
Reviewer: "Requesting changes - please address before merging"
|
||||
// PR updated to meet all standards before merge
|
||||
```
|
||||
|
||||
**The principle:**
|
||||
Every nitpick matters. Every pattern violation matters. Address them before merging, not after. We hold each other accountable for quality because cutting corners might feel faster in the moment, but it creates problems that slow everyone down later.
|
||||
|
||||
**Make it normal to challenge poor decisions, respectfully:**
|
||||
- If someone says "let's just hard-code this for now," ask "what would it take to do it the proper way the first time?"
|
||||
- If someone wants to commit untested code, push back
|
||||
- If someone suggests copying and pasting instead of creating a proper abstraction, call it out respectfully
|
||||
|
||||
Reference: [Cal.com Engineering Blog](https://cal.com/blog/engineering-in-2026-and-beyond)
|
||||
@@ -0,0 +1,67 @@
|
||||
---
|
||||
title: Maintain 80%+ Test Coverage for New Code
|
||||
impact: HIGH
|
||||
impactDescription: Prevents bugs and enables confident refactoring
|
||||
tags: testing, coverage, quality, ci
|
||||
---
|
||||
|
||||
## Maintain 80%+ Test Coverage for New Code
|
||||
|
||||
**Impact: HIGH**
|
||||
|
||||
Every PR must have near-80%+ test coverage for the code it introduces or modifies. This is enforced automatically in our CI pipeline. If you add 50 lines of new code, those 50 lines must be covered by tests. If you modify an existing function, your changes must be tested.
|
||||
|
||||
**Coverage requirements:**
|
||||
- Overall test coverage: 80%+ for new code
|
||||
- Unit test coverage: Near 100%, especially with AI assistance for generating tests
|
||||
- Global coverage tracking as a key metric that improves over time
|
||||
|
||||
**Incorrect (untested code):**
|
||||
|
||||
```typescript
|
||||
// New function with no tests
|
||||
export function calculateAvailability(user: User, date: Date): TimeSlot[] {
|
||||
// Complex logic here...
|
||||
// No corresponding test file
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (comprehensive tests):**
|
||||
|
||||
```typescript
|
||||
// calculateAvailability.ts
|
||||
export function calculateAvailability(user: User, date: Date): TimeSlot[] {
|
||||
// Complex logic here...
|
||||
}
|
||||
|
||||
// calculateAvailability.test.ts
|
||||
describe("calculateAvailability", () => {
|
||||
it("returns empty array for user with no schedule", () => {
|
||||
const user = createMockUser({ schedules: [] });
|
||||
expect(calculateAvailability(user, new Date())).toEqual([]);
|
||||
});
|
||||
|
||||
it("excludes busy times from available slots", () => {
|
||||
const user = createMockUser({
|
||||
schedules: [mockSchedule],
|
||||
busyTimes: [mockBusyTime],
|
||||
});
|
||||
const slots = calculateAvailability(user, new Date());
|
||||
expect(slots).not.toContainEqual(expect.objectContaining({
|
||||
start: mockBusyTime.start,
|
||||
}));
|
||||
});
|
||||
|
||||
it("handles timezone conversions correctly", () => {
|
||||
// Test timezone edge cases
|
||||
});
|
||||
});
|
||||
```
|
||||
|
||||
**Addressing the "coverage isn't the full story" argument:**
|
||||
Yes, we know coverage doesn't guarantee perfect tests. We know you can write meaningless tests that hit every line but test nothing meaningful. We know coverage is just one metric among many. But it's surely better to shoot for a high percentage than to have no idea where you are at all.
|
||||
|
||||
**Leverage AI for test generation:**
|
||||
AI can quickly and intelligently build comprehensive test suites. Manual testing is more and more a thing of the past.
|
||||
|
||||
Reference: [Cal.com Engineering Blog](https://cal.com/blog/engineering-in-2026-and-beyond)
|
||||
Reference in New Issue
Block a user