Common NestJS anti-patterns

After 5 years building production platforms, I've seen the same mistakes repeated across codebases—mistakes that eventually hurt performance, testability, and team velocity. Common NestJS anti-patterns emerge when developers prioritize speed over structure, and they compound quickly once a codebase reaches a certain size. Today I'm sharing the four patterns I keep refactoring out, along with the reasoning behind each fix.

Business Logic in Controllers

The controller's job is simple: accept a request, delegate to a service, and return a response. Yet I regularly find controllers with 200+ lines of database queries, calculations, validations, and orchestration baked directly into the route handler. This happens for a few reasons—developers want to "move fast," they underestimate the scope of a feature, or they've never worked on a codebase where it caused pain.

Here's what it looks like:

typescript

@Controller('orders')
export class OrdersController {
  constructor(private db: Database) {}

  @Post()
  async createOrder(@Body() dto: CreateOrderDto) {
    // Validation
    const user = await this.db.users.findUnique({ id: dto.userId });
    if (!user) throw new BadRequestException('User not found');
    if (user.isBlocked) throw new ForbiddenException('User blocked');

    // Business logic
    const inventory = await this.db.inventory.findMany({
      where: { sku: { in: dto.items.map(i => i.sku) } }
    });
    
    const orderLines = dto.items.map(item => {
      const stock = inventory.find(i => i.sku === item.sku);
      if (!stock || stock.quantity < item.qty) {
        throw new BadRequestException(`Insufficient stock for ${item.sku}`);
      }
      return {
        sku: item.sku,
        quantity: item.qty,
        price: stock.currentPrice * 0.95 // Apply loyalty discount
      };
    });

    // More calculations and updates
    const total = orderLines.reduce((sum, line) => sum + (line.price * line.quantity), 0);
    const tax = total * 0.1;
    const shipping = total > 100 ? 0 : 15;

    // Database writes
    const order = await this.db.orders.create({
      data: {
        userId: dto.userId,
        total: total + tax + shipping,
        tax,
        shipping,
        lines: { createMany: { data: orderLines } }
      }
    });

    return order;
  }
}

The problems multiply fast. You can't unit test the discount logic without spinning up a database. You can't reuse order creation in a webhook handler or batch job. You can't change pricing rules without touching the controller. The file becomes a dumping ground for everything "order-related."

Move this to a service:

typescript

@Injectable()
export class OrdersService {
  constructor(
    private db: Database,
    private inventory: InventoryService,
    private pricing: PricingService
  ) {}

  async createOrder(userId: string, items: OrderItemDto[]): Promise {
    const user = await this.validateUser(userId);
    const orderLines = await this.buildOrderLines(items);
    const calculations = this.pricing.calculateTotals(orderLines);
    
    return this.db.orders.create({
      data: {
        userId,
        ...calculations,
        lines: { createMany: { data: orderLines } }
      }
    });
  }

  private async validateUser(userId: string): Promise {
    const user = await this.db.users.findUnique({ id: userId });
    if (!user) throw new BadRequestException('User not found');
    if (user.isBlocked) throw new ForbiddenException('User blocked');
    return user;
  }

  private async buildOrderLines(items: OrderItemDto[]): Promise {
    return this.inventory.validateAndPriceItems(items);
  }
}

@Controller('orders')
export class OrdersController {
  constructor(private orders: OrdersService) {}

  @Post()
  async createOrder(@Body() dto: CreateOrderDto) {
    return this.orders.createOrder(dto.userId, dto.items);
  }
}

Now your controller is readable, testable, and single-responsibility. The service can be tested in isolation with mocked inventory and pricing. The same service works for webhooks, scheduled jobs, or admin operations.

One SharedModule Importing Everything

The SharedModule starts innocently. You need access to a logger utility in two modules, so you create a shared folder and export the logger. Months later, it exports 40 services, a dozen guards, all your DTOs, a database provider, and half your business logic. New developers don't know what goes where. Dependencies become circular. You can't reason about who depends on what.

I've seen teams where 90% of the codebase imports SharedModule out of habit, not necessity:

typescript

// shared/shared.module.ts — antipattern
@Module({
  imports: [
    PrismaModule,
    ConfigModule,
    LoggerModule,
    CacheModule,
    MailerModule
  ],
  providers: [
    PricingService,
    InventoryService,
    UserService,
    OrderService,
    PaymentService,
    NotificationService,
    DateUtilsService,
    ValidationService,
    // ... 20 more
  ],
  exports: [
    PrismaModule,
    ConfigModule,
    LoggerModule,
    CacheModule,
    MailerModule,
    PricingService,
    InventoryService,
    // ... export everything
  ]
})
export class SharedModule {}

This breeds lazy thinking. A new feature module just imports SharedModule and assumes everything it needs is there. No one knows what's actually required. Dependency trees become invisible.

Instead, create focused modules for cross-cutting concerns:

typescript

// infrastructure/database/database.module.ts
@Module({
  providers: [PrismaService],
  exports: [PrismaService]
})
export class DatabaseModule {}

// infrastructure/config/config.module.ts
@Module({
  imports: [ConfigModule.forRoot()],
  exports: [ConfigModule]
})
export class AppConfigModule {}

// shared/logging/logging.module.ts
@Module({
  providers: [LoggerService],
  exports: [LoggerService]
})
export class LoggingModule {}

// features/orders/orders.module.ts
@Module({
  imports: [
    DatabaseModule,
    LoggingModule,
    PricingModule,
    InventoryModule
  ],
  controllers: [OrdersController],
  providers: [OrdersService],
  exports: [OrdersService]
})
export class OrdersModule {}

Now each module explicitly declares its dependencies. You can trace exactly what a feature needs. Testing becomes easier because you only import what matters. New team members can understand module boundaries without archaeology.

Request-Scoped Providers Everywhere

Request-scoped providers are useful when you need a fresh instance per HTTP request—think request ID tracking or multi-tenancy. But I see teams making every service request-scoped out of caution, "just in case." This kills performance.

Each request creates a new instance of every request-scoped dependency in its chain. With 100 concurrent requests and 10 request-scoped services per request, you're creating 1,000 service instances. Memory pressure spikes, garbage collection stutters, latency climbs.

typescript


// antipattern
@Injectable({ scope: Scope.REQUEST })
export class OrdersService {
  constructor(private db: Database) {}
  // ...
}

@Injectable({ scope: Scope.REQUEST })
export class InventoryService {
  constructor(private db: Database) {}
  // ...
}

@Injectable({ scope: Scope.REQUEST })
export class PricingService {
  // ...
}

Use request scope only when you genuinely need per-request state. A user ID, tenant ID, request-local cache, correlation ID—these make sense. A database connection pool, a pricing engine, a validation utility—these should be singletons.

typescript

// better
@Injectable({ scope: Scope.REQUEST })
export class RequestContextService {
  userId: string;
  tenantId: string;
  requestId: string;
}

@Injectable() // singleton
export class PricingService {
  constructor(private context: RequestContextService) {}
  
  getPrice(sku: string): number {
    // Use context.tenantId to fetch tenant-specific pricing
    // but this service instance is shared across requests
  }
}

@Injectable() // singleton
export class OrdersService {
  constructor(
    private db: Database,
    private pricing: PricingService,
    private context: RequestContextService
  ) {}
}

In this approach, the RequestContextService is lightweight and per-request. The heavy lifters—database, pricing logic, services—are singletons that know how to read from the request context when needed.

Any Types in DTOs and Services

TypeScript's any type disables type checking. I see it used in DTOs and service parameters as a shortcut—"I don't want to define the structure, so I'll just use any"—and it erodes safety fast.

typescript

// antipattern
export class CreateOrderDto {
  userId: string;
  items: any; // Could be anything
  metadata: any; // Guessing game
}

@Injectable()
export class OrdersService {
  createOrder(userId: string, payload: any): Promise {
    // No idea what payload contains
    // No idea what this returns
  }
}

When someone calls ordersService.createOrder('123', something), there's no contract. IDEs can't autocomplete. Refactoring breaks silently. You discover bugs in production when payload.items turns out to be undefined.

Define the shape explicitly, even if it's verbose:

typescript

export class OrderItemDto {
  sku: string;
  quantity: number;
}

export class OrderMetadataDto {
  source: 'web' | 'mobile' | 'api';
  campaignId?: string;
  notes?: string;
}

export class CreateOrderDto {
  userId: string;
  items: OrderItemDto[];
  metadata: OrderMetadataDto;
}

export class OrderResponseDto {
  id: string;
  userId: string;
  total: number;
  createdAt: Date;
  status: 'pending' | 'confirmed' | 'shipped' | 'delivered';
}

@Injectable()
export class OrdersService {
  async createOrder(
    userId: string,
    payload: CreateOrderDto
  ): Promise {
    // Full type safety, IDE support, clear contract
  }
}

Yes, there's more code. But you catch mistakes at compile time, not runtime. Your API surface is clear. Refactoring is safe. It's the trade-off of building robust systems.

Common Mistakes

Mixing abstraction levels. Putting HTTP validation, business logic, and database queries in the same function. Each layer has one job.
Treating SharedModule as a dumping ground. It grows without discipline and becomes a dependency black hole. Be ruthless about what lives there.
Assuming request scope is safe by default. It's an optimization problem. Measure memory and latency before defaulting to request scope.
Skipping DTOs to save time. You lose the time saved when someone passes the wrong shape and breaks production.
Not testing services in isolation. Controllers should be thin enough that you rarely need to test them. Services should run without a database or HTTP layer.

Key Takeaways

Controllers handle HTTP concerns—routing, status codes, headers. Business logic belongs in services.
Organize modules by feature or domain, not by putting everything in one shared bucket. Explicit dependencies reveal architecture.
Use request scope sparingly, only for per-request state like user ID or tenant ID. Keep services, utilities, and data access singletons.
Type everything in DTOs and service signatures. The few extra lines of DTO code prevent silent failures and enable refactoring.
Refactoring these patterns is easier early. Catch them in code review before they compound across a growing codebase.

---

Common NestJS anti-patterns: what developers should know