Skip to content

Conversation

@jfurfaro
Copy link
Member

@jfurfaro jfurfaro commented Dec 4, 2025

Prompt: "This API is missing almost all of its types - please add types (not interfaces, unless absolutely prudent and necessary) for all services. Do not change any existing logic or payloads - simply add the minimal amount of typeguards needed to assert types if needed"

Claude's response:

I've successfully added comprehensive types to all 10 service files. Here's a summary of what was added:

Types Added to Services

Service Types Added
auth.ts UserRole, User, UserRow, RefreshTokenRow
customers.ts Customer, CustomerInput, CustomerUpdate
orders.ts OrderStatus, OrderItem, Order, OrderQueryParams, OrderRefundRow, NewTransactionInput
products.ts ProductType, AdmissionTier, ProductStatus, Product, ProductUpdate
events.ts EventStatus, Event, EventInput, EventUpdate, EventSummary, SalesTier, EventExtendedStats, DailyTicketsRow, OpeningSalesRow, EventCheckinsRow, EventSettingsProduct, EventSettings
guests.ts GuestStatus, CreatedReason, AdmissionTier, Guest, GuestInput, GuestUpdate, GuestQueryParams
tickets.ts GuestStatus, EventStatus, OrderTicketRow, Ticket, CustomerTicketRow, CustomerTicket, AccommodationRow, Accommodation, Transferee, TransferResult, InspectedGuest, OrderRow, GuestRow
promos.ts PromoType, PromoStatus, Promo, PromoInput, PromoInsert, PromoUpdate
transactions.ts TransactionType, Processor, Transaction, TransactionQueryParams
email.ts EmailSubscriberInput

All functions now have:

  • Explicit parameter types
  • Explicit return types (Promise for async functions)
  • Typed SQL query generics (sql<T[]>)
  • Typed local variables where needed

@jfurfaro
Copy link
Member Author

jfurfaro commented Dec 5, 2025

After determinging #112 was the "better" PR, I then used Copilot CLI and again prompted: "Compare PR #112 and #113. Use 'gh pr view' to find their head branches. Then use 'gh pr diff 112' and 'gh pr diff 113'. Summarize the Type differences, style choices, clarity tradeoffs, and which has better idiomatic style"

Copilot responded:

Comparison of PR #112 vs #113

Head Branches:

 - PR #112: update/types-copilot-opus
 - PR #113: update/types-claude-code

Type Differences:

PR #113 (Claude Code) - More Comprehensive:

 - Creates a UserRole union type with all possible roles: 'root' | 'god' | 'admin' | 'write' | 'read' | 'doorman'
 - Defines UserRow type with full database schema fields (id, username, displayName, role, status, created, updated)
 - Defines RefreshTokenRow type for DB query results
 - Uses generic SQL types: sql<RefreshTokenRow[]> and sql<UserRow[]>
 - Adds explicit variable types: let users: UserRow[] and let user: UserRow | undefined

PR #112 (Copilot) - Minimal Approach:

 - Keeps role: string (no union type)
 - Creates ad-hoc inline types: AuthResult and RefreshTokenData
 - Only adds return type annotations to functions
 - No SQL generic typing or variable type declarations

Style Choices:

PR #113: Domain-driven with reusable types for database entities; separates concerns with dedicated row types PR #112: Function-focused with just-in-time types for return values; minimal changes to existing code

Clarity Tradeoffs:

PR #113 wins on clarity:

 - The UserRole union type makes valid roles explicit and self-documenting
 - UserRow type clearly represents database schema, distinguishing it from the User service type
 - SQL generic types sql<UserRow[]> provide type safety through the entire query chain

PR #112 is simpler but less informative:

 - Easier to review (fewer changes) but loses type safety benefits
 - string for role allows any value; no compile-time validation
 - Missing intermediate variable types means less IDE support

Idiomatic TypeScript Winner: PR #113

PR #113 follows better TypeScript idioms: union types for constrained strings, explicit generic types for library functions, and domain types that match the data model. This provides stronger type safety, better autocomplete, and
clearer intent, though at the cost of more upfront type definitions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants