-
Notifications
You must be signed in to change notification settings - Fork 0
Centralize weight use cases in DI container #1453
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: marcuscastelo/issue1447
Are you sure you want to change the base?
Centralize weight use cases in DI container #1453
Conversation
- Refactor createWeightUseCases to accept granular authDeps instead of whole useCases object, breaking circular dependency - Add WeightUseCases to useCases.ts container - Add WeightUseCases type to container.tsx - Keep backward-compatible shim in weightUseCases.ts for legacy consumers - Next: migrate consumers to use container instead of direct imports
…iner Migrate all consumers of weightUseCases to use the centralized DI container instead of direct imports from the module: - weightChartUseCases.ts - now uses useCases.weightUseCases() - macroTargetUseCases.ts - now uses useCases.weightUseCases() - WeightView.tsx - now uses useCases.weightUseCases() - WeightEvolution.tsx - now uses useCases.weightUseCases() - MacroEvolution.tsx - now uses useCases.weightUseCases() - MacroProfile.tsx - now uses useCases.weightUseCases() - BodyMeasureChart.tsx - now uses useCases.weightUseCases() - RestoreProfileModal.tsx - now uses useCases.weightUseCases() This prepares for removal of the backward-compatible shim in weightUseCases.ts
All consumers now use the centralized DI container (useCases.weightUseCases()), so the shim export is no longer needed. Removed: - export const weightUseCases = createWeightUseCases() - export function refetchUserWeights() The factory createWeightUseCases() and type WeightUseCases remain exported for DI wiring and type safety.
… circular import - Remove useCases import from weightUseCases.ts to break circular dependency - Make authDeps a required parameter for createWeightUseCases - Update container.tsx to provide authDeps when creating default weight use-cases This breaks the circular dependency that was causing test failures when modules imported from useCases.ts at module initialization time.
- Document weightUseCases shim removal as complete - Document clipboardUseCases circular dependency blocking centralization - List remaining shims to evaluate - Add commit references for Batch 7
- Refactor createWeightChartUseCases to accept granular WeightChartDeps instead of typeof useCases to avoid circular dependency - Remove weightChartUseCases shim (all consumers now use container) - Update consumers (WeightChartTooltip, WeightEvolution) to use useCases.weightChartUseCases() - Add weightChartUseCases to useCases.ts with proper dependency wiring
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR centralizes weight and weight-chart use-cases in the DI container to eliminate circular dependencies and remove backward-compatible shims. The refactoring introduces granular dependency interfaces (WeightUseCasesDeps, WeightChartDeps) that accept only the minimal required dependencies instead of the full useCases object, breaking circular import chains. All consumers (8 files) are migrated to access these use-cases through useCases.weightUseCases() and useCases.weightChartUseCases().
Key changes:
- Removed circular
useCasesimport fromweightUseCases.tsby makingauthDepsa required parameter - Centralized both
weightUseCasesandweightChartUseCasesin the DI container - Removed backward-compatible shims from both factories
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
src/di/useCases.ts |
Added weightUseCases and weightChartUseCases to centralized container, wired with granular dependencies to avoid circular imports |
src/di/container.tsx |
Extended Container type with WeightUseCases, created default weight use-cases with auth dependencies |
src/modules/weight/application/weight/usecases/weightUseCases.ts |
Made authDeps required parameter, removed circular useCases import, removed backward-compatible shim |
src/modules/weight/application/chart/weightChartUseCases.ts |
Introduced WeightChartDeps interface, made dependencies required, removed shim |
src/sections/weight/components/WeightView.tsx |
Migrated to use useCases.weightUseCases() for update/delete operations |
src/sections/weight/components/WeightEvolution.tsx |
Migrated to use useCases.weightUseCases() and useCases.weightChartUseCases() - has critical accessor wrapping bug |
src/sections/weight/components/WeightChartTooltip.tsx |
Migrated to use useCases.weightChartUseCases() for progress calculation |
src/sections/profile/measure/components/BodyMeasureChart.tsx |
Migrated to use useCases.weightUseCases().weights() in measure processing |
src/sections/profile/components/MacroProfile.tsx |
Migrated to use useCases.weightUseCases().latest() for weight check |
src/sections/profile/components/MacroEvolution.tsx |
Migrated all chart components to use useCases.weightUseCases() - has critical accessor wrapping bug |
src/modules/diet/macro-profile/ui/RestoreProfileModal.tsx |
Migrated to use useCases.weightUseCases() for weight lookups |
src/modules/diet/macro-target/application/macroTargetUseCases.ts |
Updated default to use centralized useCases.weightUseCases(), but maintains shim pattern that may cause circular dependency |
DI-migration-plan.md |
Updated with Batch 7 progress, documents completed migrations and remaining work |
| <WeightChart | ||
| weights={weightUseCases.weights} | ||
| desiredWeight={weightChartUseCases.desiredWeight()} | ||
| weights={() => useCases.weightUseCases().weights()} |
Copilot
AI
Dec 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The weights prop is being wrapped in an arrow function () => useCases.weightUseCases().weights(), but WeightChart expects weights to be an Accessor<readonly Weight[]>. Since useCases.weightUseCases().weights is already an accessor (a signal getter function), wrapping it in another arrow function creates a double-wrapped function.
This should be: weights={useCases.weightUseCases().weights} without the extra arrow function wrapper.
| weights={() => useCases.weightUseCases().weights()} | |
| weights={useCases.weightUseCases().weights} |
| // Note: Shim removed - all consumers should use the centralized DI container. | ||
| // Import via `useCases.weightChartUseCases()` from '~/di/useCases'. | ||
|
|
Copilot
AI
Dec 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment says "Shim removed" and instructs users to import via useCases.weightChartUseCases(), but the instructions should clarify that consumers access this by calling useCases.weightChartUseCases() to get the instance. Consider rephrasing to: "Note: Shim removed - all consumers should access via useCases.weightChartUseCases() from '~/di/useCases'." for clarity.
| // Note: Shim removed - all consumers should use the centralized DI container. | |
| // Import via `useCases.weightChartUseCases()` from '~/di/useCases'. | |
| // Note: Shim removed - all consumers should access via useCases.weightChartUseCases() from '~/di/useCases'. |
DI-migration-plan.md
Outdated
| - `recipeItemUseCases` | ||
| - `macroOverflowUseCases`, `macroTargetUseCases` | ||
| - `mealUseCases` | ||
| - `weightChartUseCases` |
Copilot
AI
Dec 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The line listing weightChartUseCases should be removed from the "Remaining shims to evaluate" section since it was already completed and documented in the "Completed" section above (lines 118-120).
| - `weightChartUseCases` |
Co-authored-by: marcuscastelo <27441558+marcuscastelo@users.noreply.github.com>
Co-authored-by: marcuscastelo <27441558+marcuscastelo@users.noreply.github.com>
Co-authored-by: marcuscastelo <27441558+marcuscastelo@users.noreply.github.com>
…pletion) Co-authored-by: marcuscastelo <27441558+marcuscastelo@users.noreply.github.com>
…ompatibility-shims refactor(di): Remove 4 backward-compatible shims (Batch 7 partial)
…rt factories Co-authored-by: marcuscastelo <27441558+marcuscastelo@users.noreply.github.com>
…se cases directly
…ories Co-authored-by: marcuscastelo <27441558+marcuscastelo@users.noreply.github.com>
…Repository Co-authored-by: marcuscastelo <27441558+marcuscastelo@users.noreply.github.com>
…ule-exports refactor(food): add explicit return types to repository helpers
….com/marcuscastelo/macroflows into marcuscastelo/issue1447-remove-shims
…ood-infra-exports refactor(recent-food): Encapsulate infrastructure — export only factories
….com/marcuscastelo/macroflows into marcuscastelo/issue1447-remove-shims
…ipe-infra refactor(diet/recipe): Encapsulate infrastructure exports — export only factories
…ile modules Co-authored-by: marcuscastelo <27441558+marcuscastelo@users.noreply.github.com>
Co-authored-by: marcuscastelo <27441558+marcuscastelo@users.noreply.github.com>
….com/marcuscastelo/macroflows into marcuscastelo/issue1447-remove-shims
…d-compatibility-shims refactor(di): Remove backward-compatibility shims from telemetry, search, profile, and recentFood modules
….com/marcuscastelo/macroflows into marcuscastelo/issue1447-remove-shims
refactor(supabase): implement no-op storage for SSR environments
Refactor weight use cases to be centralized in the DI container, removing backward-compatible shims and breaking circular dependencies. Migrate all consumers to utilize the new structure, ensuring type safety and preparing for future enhancements. Document progress and remaining tasks in the migration plan.