refactor(diet/recipe): Encapsulate infrastructure exports — export only factories#1457
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…rt factories Co-authored-by: marcuscastelo <27441558+marcuscastelo@users.noreply.github.com>
36a839f
into
marcuscastelo/issue1447-remove-shims
There was a problem hiding this comment.
Pull request overview
This PR refactors the diet/recipe module infrastructure to improve encapsulation by restricting exports to factory functions only, aligning with the repository's Clean Architecture approach.
Key Changes:
- Made 6 helper functions in
recipeRepository.tsinternal, exposing onlycreateRecipeRepository()factory - Changed DTO types in
supabaseRecipeMapper.tsfrom exported to internal (file-local) - Wrapped realtime service functions in
createRecipeRealtimeService()factory
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/modules/diet/recipe/infrastructure/recipeRepository.ts |
Internalized 6 repository helper functions (fetchUserRecipes, fetchRecipeById, etc.), maintaining only factory export |
src/modules/diet/recipe/infrastructure/supabase/supabaseRecipeMapper.ts |
Changed 3 DTO types (RecipeDTO, InsertRecipeDTO, UpdateRecipeDTO) from exported to internal |
src/modules/diet/recipe/infrastructure/supabase/realtime.ts |
Wrapped setupRecipeRealtimeSubscription and initializeRecipeRealtime in createRecipeRealtimeService() factory, encapsulating initialized state in closure |
| export function createRecipeRealtimeService() { | ||
| let initialized = false | ||
|
|
||
| /** | ||
| * Sets up granular realtime subscription for recipe changes | ||
| * @param onRecipeChange - Callback for granular updates with event details | ||
| */ | ||
| export function setupRecipeRealtimeSubscription( | ||
| onRecipeChange: (event: { | ||
| eventType: 'INSERT' | 'UPDATE' | 'DELETE' | ||
| old?: Recipe | ||
| new?: Recipe | ||
| }) => void, | ||
| ): void { | ||
| registerSubapabaseRealtimeCallback( | ||
| SUPABASE_TABLE_RECIPES, | ||
| recipeSchema, | ||
| onRecipeChange, | ||
| ) | ||
| } | ||
|
|
||
| export function initializeRecipeRealtime(): void { | ||
| if (initialized) { | ||
| return | ||
| /** | ||
| * Sets up granular realtime subscription for recipe changes | ||
| * @param onRecipeChange - Callback for granular updates with event details | ||
| */ | ||
| function setupRecipeRealtimeSubscription( | ||
| onRecipeChange: (event: { | ||
| eventType: 'INSERT' | 'UPDATE' | 'DELETE' | ||
| old?: Recipe | ||
| new?: Recipe | ||
| }) => void, | ||
| ): void { | ||
| registerSubapabaseRealtimeCallback( | ||
| SUPABASE_TABLE_RECIPES, | ||
| recipeSchema, | ||
| onRecipeChange, | ||
| ) | ||
| } | ||
| logging.debug(`Recipe realtime initialized!`) | ||
| initialized = true | ||
| registerSubapabaseRealtimeCallback( | ||
| SUPABASE_TABLE_RECIPES, | ||
| recipeSchema, | ||
| (event) => { | ||
| logging.debug(`Event:`, event) | ||
|
|
||
| switch (event.eventType) { | ||
| case 'INSERT': { | ||
| if (event.new !== undefined) { | ||
| recipeCacheStore.upsertToCache(event.new) | ||
| function initializeRecipeRealtime(): void { | ||
| if (initialized) { | ||
| return | ||
| } | ||
| logging.debug(`Recipe realtime initialized!`) | ||
| initialized = true | ||
| registerSubapabaseRealtimeCallback( | ||
| SUPABASE_TABLE_RECIPES, | ||
| recipeSchema, | ||
| (event) => { | ||
| logging.debug(`Event:`, event) | ||
|
|
||
| switch (event.eventType) { | ||
| case 'INSERT': { | ||
| if (event.new !== undefined) { | ||
| recipeCacheStore.upsertToCache(event.new) | ||
| } | ||
| break | ||
| } | ||
| break | ||
| } | ||
|
|
||
| case 'UPDATE': { | ||
| if (event.new) { | ||
| recipeCacheStore.upsertToCache(event.new) | ||
| case 'UPDATE': { | ||
| if (event.new) { | ||
| recipeCacheStore.upsertToCache(event.new) | ||
| } | ||
| break | ||
| } | ||
| break | ||
| } | ||
|
|
||
| case 'DELETE': { | ||
| if (event.old) { | ||
| recipeCacheStore.removeFromCache({ by: 'id', value: event.old.id }) | ||
| case 'DELETE': { | ||
| if (event.old) { | ||
| recipeCacheStore.removeFromCache({ | ||
| by: 'id', | ||
| value: event.old.id, | ||
| }) | ||
| } | ||
| break | ||
| } | ||
| break | ||
| } | ||
| } | ||
| }, | ||
| ) | ||
| }, | ||
| ) | ||
| } | ||
|
|
||
| return { | ||
| setupRecipeRealtimeSubscription, | ||
| initializeRecipeRealtime, | ||
| } | ||
| } |
There was a problem hiding this comment.
The realtime service has been wrapped in a factory pattern (createRecipeRealtimeService()), which is inconsistent with other realtime services in the codebase (e.g., day-diet/infrastructure/supabase/realtime.ts and macro-profile/infrastructure/supabase/realtime.ts).
These other modules export the initialization function directly with module-level singleton initialized state, not wrapped in a factory. The factory pattern here could break singleton behavior if multiple instances are created, potentially causing duplicate subscriptions.
Recommendation: Export initializeRecipeRealtime and setupRecipeRealtimeSubscription directly with module-level initialized state to match the established pattern in the codebase.
Infrastructure files were exporting individual helper functions alongside factories, leaking implementation details. This refactoring restricts exports to factory functions only.
Changes
recipeRepository.tsfetchUserRecipes,fetchRecipeById, etc.)createRecipeRepository()factory is exportedsupabaseRecipeMapper.tsRecipeDTO,InsertRecipeDTO,UpdateRecipeDTO)supabaseRecipeMapperobject exportrealtime.tscreateRecipeRealtimeService()factoryinitializedstate within factory closureNo changes
supabaseRecipeGateway.tsalready followed patternrecipeCacheStore.tsintentionally exports signal store (not a helper function)API Surface
Before:
After:
All consumers already use the factory pattern, so no breaking changes.
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.