-
Notifications
You must be signed in to change notification settings - Fork 0
refactor(di): Remove backward-compatibility shims from telemetry, search, profile, and recentFood modules #1461
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…ile modules Co-authored-by: marcuscastelo <27441558+marcuscastelo@users.noreply.github.com>
Co-authored-by: marcuscastelo <27441558+marcuscastelo@users.noreply.github.com>
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 removes backward-compatibility shims from 4 modules (Telemetry, CachedSearch, Profile, and RecentFood) as part of the ongoing DI migration. The refactor eliminates legacy named exports and requires consumers to explicitly instantiate factories. While the direction is correct and aligns with the migration plan, there is a critical bug in the Profile module changes where two components create separate instances of stateful reactive signals, breaking state synchronization.
Key changes:
- Removed shims from Telemetry, CachedSearch, Profile, and RecentFood modules
- Updated 13 consumers to instantiate factories directly at module level
- Updated test mocks to work with factory pattern instead of named exports
Critical Issue:
The Profile module uses createRoot() to create reactive signals. When UserInfo.tsx and UserInfoCapsule.tsx each create their own profile instance at module level, they get completely isolated state that won't stay synchronized. This breaks the expected behavior where both components should share the same profile state.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
src/modules/observability/application/telemetry.ts |
Removed backward-compatible shim; consumers now call factory directly |
src/entry-client.tsx |
Updated to instantiate telemetry factory |
src/entry-server.tsx |
Updated to instantiate telemetry factory |
src/instrument.server.ts |
Updated to instantiate telemetry factory |
src/modules/search/application/usecases/cachedSearchCrud.ts |
Removed shim and legacy named exports |
src/modules/diet/food/application/usecases/foodCrud.ts |
Updated to instantiate cachedSearchCrud factory |
src/modules/diet/food/infrastructure/api/application/apiFood.ts |
Updated to instantiate cachedSearchCrud factory |
src/modules/profile/application/profile.ts |
Removed backward-compatible shim |
src/sections/profile/components/UserInfo.tsx |
Creates separate profile instance at module level (broken state) |
src/sections/profile/components/UserInfoCapsule.tsx |
Creates separate profile instance at module level (broken state) |
src/modules/recent-food/application/usecases/recentFoodCrud.ts |
Removed shim and legacy named exports |
src/sections/search/components/TemplateSearchModal.tsx |
Updated to instantiate recentFoodCrud factory (creates duplicate instance) |
src/sections/common/components/buttons/RemoveFromRecentButton.tsx |
Updated to instantiate recentFoodCrud factory (creates duplicate instance) |
src/sections/common/components/buttons/RemoveFromRecentButton.test.tsx |
Updated test mocks to work with factory pattern |
src/modules/template-search/application/usecases/templateSearchState.ts |
Updated to accept RecentFoodCrud dependency via factory |
305c96f
into
marcuscastelo/issue1447-remove-shims
Problem
DI migration left backward-compatibility shims (legacy named exports and wrapper functions) that add runtime indirection, hide module boundaries, and increase maintenance overhead. This PR begins systematic removal of all shims.
Changes
Completed: 4 of 19 modules (21%)
Phase 1: Utility Modules
isSearchCached,markSearchAsCached, etc.)Phase 2a: RecentFood
fetchUserRecentFoods,deleteRecentFoodByReference, etc.)Pattern
Before (with shim):
After (factory only):
Remaining
15 modules across 5 phases:
Deferred:
Final phase: integrate all factories into
src/di/container.tsxand removesrc/di/useCases.tsglobal instance.Original prompt
This section details on the original issue you should resolve
<issue_title>Refactor(di): Remove all DI backward-compatibility shims and complete migration plan</issue_title>
<issue_description># Refactor Template
Title:
Refactor(di): Remove all DI backward-compatibility shims and complete migration plan
Description:
Over the past weeks several migration batches moved module factories and use-cases into the DI container while leaving short-lived backward-compatibility "shims" (legacy named exports, helper re-exports, and runtime indirection) in place. Those shims increase runtime indirection, hide module boundaries, and create testing and maintenance friction. This refactor finishes the DI migration by removing all remaining shims, consolidating factories and exports in the DI container, and updating consumers and tests to use the container/factories directly.
This issue codifies a step-by-step migration plan, lists all affected modules/files discovered in the last 50 commits, and defines acceptance criteria to safely remove the shims and keep CI/tests passing.
Scope:
Affected modules and files (collected from the last 50 commits — review each for leftover shims and migration impact):
(Note: Some files above were already migrated in batches; this list is exhaustive for the last 50 commits and helps verify no leftover shims remain.)
Motivation:
Migration Plan (step-by-step):
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.