Refactor dependency injection and implement backward compatibility#1450
Refactor dependency injection and implement backward compatibility#1450marcuscastelo merged 82 commits intorc/v0.16.0from
Conversation
- Converted userUseCases to factory and added backward-compatible shim\n- Wired default userUseCases and authUseCases into src/di/container.tsx using Supabase repo defaults\n- Ran project checks (lint/ts/tests) locally
- Added authUseCases shim for backward compatibility using existing userUseCases\n- Exported AuthUseCases type
- Converted recipe CRUD functions to factory-based DI\n- Added backward-compatible shims delegating to default factory instance\n- Exported RecipeCrud type
- Converted food CRUD to factory-based DI\n- Added default shim delegating to Supabase food repository\n- Exported FoodCrud type
- Introduced createMealUseCases with dayUseCases dependency\n- Added backward-compatible shim and exported MealUseCases type
- Exported DayUseCases type from dayUseCases\n- Adjusted meal factory to accept typed DayUseCases and added shim
- Replaced any types with DayUseCases import to satisfy lint rules
…shims - Converted macro profile CRUD service and usecases to factories\n- Added default shims to preserve compatibility\n- Exported types
- Converted macro-profile CRUD service and usecases to factories and added default shims\n- Removed unused imports to satisfy lint
- Converted dayUseCases to createDayUseCases factory that encloses signals in createRoot\n- Kept backward-compatible dayUseCases shim\n- Exported DayUseCases type
- Removed duplicate DayUseCases type export
…(frequent commits)
…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
…roject-specific settings
…emove-shims Centralize weight use cases in DI container
…tests to use factory (one commit per shim removal)
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
…lify AppMode type
…ases module and shims
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 118 out of 119 changed files in this pull request and generated 7 comments.
Comments suppressed due to low confidence (1)
src/sections/search/components/TemplateSearchModal.tsx:23
recentFoodCrudis instantiated at module scope and there are furtherimportdeclarations after it. Please keep imports grouped at the top and create/injectrecentFoodCrudinside the component/container to avoid module-scope side effects and ordering surprises.
| const getMacroOverflow = (deps: Parameters<typeof createMacroOverflow>[0]) => { | ||
| const memo = createMemo<ReturnType<typeof createMacroOverflow>>(() => | ||
| createMacroOverflow(deps), | ||
| ) | ||
|
|
There was a problem hiding this comment.
getMacroOverflow builds a new createMemo accessor each time it’s called. In Solid, reactive primitives shouldn’t be created inside other computations/recomputations like this. Create the macro-overflow use-cases once at component init and reuse them.
| return getMacroOverflow({ | ||
| dayUseCases: useCases.dayUseCases(), | ||
| macroTargetUseCases: useCases.macroTargetUseCases(), | ||
| })().isOverflow({ item, originalItem }) |
There was a problem hiding this comment.
This creates a memo-backed overflow checker inside isMacroOverflowing’s computation. On recompute, this can create orphaned computations. Prefer const macroOverflow = createMacroOverflow(...) (or a single memo) defined once, then call macroOverflow.isOverflow(...) here.
| // Clean expired entries every hour and refresh signal | ||
| setInterval( | ||
| () => { | ||
| store.cleanExpired() | ||
| }, | ||
| 60 * 60 * 1000, | ||
| ) |
There was a problem hiding this comment.
The setInterval started here is never cleared. This can leak timers in tests/HMR and when multiple containers are created. Track the interval id and clear it via onCleanup/a returned dispose function.
| describe('Tab Visibility Logic', () => { | ||
| it('respects debouncedTab state for component visibility', () => { | ||
| // Test when tab is 'recent' | ||
| mockDebouncedTab.mockReturnValue('recent') | ||
| expect(debouncedTab()).toBe('recent') | ||
| const visibleTabs = ['recent', 'all'] as const | ||
|
|
||
| // Test when tab is not 'recent' | ||
| mockDebouncedTab.mockReturnValue('all') | ||
| expect(debouncedTab()).toBe('all') | ||
| expect(visibleTabs[0]).toBe('recent') |
There was a problem hiding this comment.
This test no longer verifies the real tab-visibility logic (it asserts a hardcoded array instead of the component/state behavior). Either remove it or rewrite it to mock templateSearchState.debouncedTab() and assert visibility for 'recent' vs other tabs.
| calculateWeightProgress, | ||
| diet, | ||
| isMobile = false, |
There was a problem hiding this comment.
buildWeightChartOptions now requires calculateWeightProgress and diet, and the tooltip wiring depends on those inputs. The previous formatter tests were removed; please add/restore unit tests covering the y-axis formatter edge cases and tooltip parameter wiring to prevent regressions.
| const clipboardStore = | ||
| deps?.clipboardStore ?? | ||
| createRoot(() => { | ||
| // Default to RAM-only (no persistence) | ||
| const store = localCreateClipboardStore({ |
There was a problem hiding this comment.
createClipboardUseCases creates a nested createRoot when deps.clipboardStore is not provided. Since the returned dispose is not exposed, this root cannot be cleaned up and will leak owners across container/test lifecycles. Prefer creating the store under the caller’s existing root (remove the nested createRoot) or return a dispose the container can call.
| const latest = useCases.clipboardUseCases().read() | ||
| const all = useCases.clipboardUseCases().readAll() |
There was a problem hiding this comment.
The README example uses useCases.clipboardUseCases().read() / .readAll(), but the current use-cases API exposes fetchLatest() and entries() (no read/readAll). Update the example to match the actual API to avoid broken copy/paste usage.
| const latest = useCases.clipboardUseCases().read() | |
| const all = useCases.clipboardUseCases().readAll() | |
| const latest = useCases.clipboardUseCases().fetchLatest() | |
| const all = useCases.clipboardUseCases().entries() |
Introduce a dependency injection container and refactor various use cases to utilize factory patterns. Remove unnecessary commands from the test script.
Fix #1447