Skip to content

Conversation

@jfurfaro
Copy link
Member

@jfurfaro jfurfaro commented Dec 2, 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"

@jfurfaro
Copy link
Member Author

jfurfaro commented Dec 4, 2025

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

Copilot responded:

Summary: PR #111 vs PR #112 Comparison

Type Differences

PR #111 (Codex):

 - Uses inline object types: Promise<{accessToken: string; refreshToken: string}>
 - Uses inline anonymous types for return values
 - Less type declaration overhead
 - Types are minimal and directly in function signatures

PR #112 (Copilot/Opus):

 - Creates named type aliases: type AuthResult = {...}
 - More comprehensive type definitions with separate declarations
 - More extensive "Raw" types for DB transformations (e.g., EventRaw, TransactionRaw)
 - Includes intermediate types like CustomerTicketRow, AccommodationRow

Style Choices

PR #111:

 - Lighter approach: Inline types, less abstraction
 - Uses intersection types with Pick/Omit utilities
 - Variable typing: let user: User (assumes non-null)
 - Fewer intermediate types

PR #112:

 - Heavier approach: Extensive named types for every shape
 - More consistent | undefined usage: let user: User | undefined
 - Better null safety with explicit undefined handling
 - Creates types for DB row shapes vs domain types
 - More verbose but comprehensive

Clarity Tradeoffs

PR #111 Advantages:

 - Faster to scan - types are where they're used
 - Less cognitive overhead navigating type definitions
 - Cleaner for simple cases

PR #111 Disadvantages:

 - Inline object types harder to reuse or reference
 - Less self-documenting

PR #112 Advantages:

 - Named types are self-documenting
 - Clear separation between raw DB types and domain types
 - Better for refactoring and reuse
 - More explicit about nullability

PR #112 Disadvantages:

 - More verbose - lots of type declarations to parse
 - Can feel over-engineered for simple functions

Which Has Better Idiomatic TypeScript?

Winner: PR #112 - It follows more idiomatic TypeScript patterns:

 - Named types over inline: TypeScript conventions favor named types for clarity and reuse
 - Explicit nullability: Using | undefined is more correct TypeScript practice
 - Separation of concerns: Raw DB types vs domain types is a best practice
 - Type safety: Better handling of optional values and type narrowing
 - Boolean coercion: PR #112 uses Boolean() wrapper (line 112) which is more explicit than PR #111's bitwise operator

However, PR #111 is more pragmatic and may be preferred in fast-moving codebases where the extra type ceremony isn't worth the maintenance burden. PR #112 is better for larger teams and long-term maintainability.

@jfurfaro
Copy link
Member Author

jfurfaro commented Dec 8, 2025

Closing in favor of Opus 4.5

@jfurfaro jfurfaro closed this Dec 8, 2025
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