-
Notifications
You must be signed in to change notification settings - Fork 0
Refactor Copilot instructions and enhance dependency injection #1439
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
Remove sections that referenced memory retrieval, memory updates, and the requirement to begin chats with "Remembering..." from the Copilot instructions file.
Clarify barrel file prohibition and prefer direct imports for tree-shaking. Require verifying referenced scripts and prefer repo-specific scripts. Add retry and escalation steps for `npm run copilot:check` instead of blocking indefinitely. Relax `reportedBy` to be required only for machine-readable/auditable outputs. Improve solo-project, JSDoc, and import guidance for clearer, actionable rules.
|
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 introduces a significant architectural refactoring that replaces direct singleton imports with a centralized dependency injection (DI) container pattern, alongside improvements to editor configuration, accessibility, and Copilot instructions.
Key changes:
- Introduces a new DI container (
~/di/useCases) that provides factory functions for use cases (auth, user, guest) - Migrates all components and modules from direct singleton imports to DI container access
- Adds accessibility improvements (button
typeattributes, SVG<title>elements, proper label-input associations) - Updates editor configurations for non-relative imports and debugging support
Reviewed changes
Copilot reviewed 49 out of 51 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
src/di/useCases.ts |
New DI container - Centralizes use case creation with potential circular dependency and memoization issues |
src/modules/auth/application/authDI.ts |
New auth DI module - Wraps auth initialization in createRoot with proper reactive context |
src/modules/auth/application/usecases/authUseCases.ts |
Refactored to factory function pattern accepting DI dependencies |
src/modules/auth/application/services/authService.ts |
Now accepts user use cases via DI instead of direct import |
src/shared/guest/guestDI.ts |
New guest DI module - Missing createRoot wrapper for createEffect |
src/shared/guest/guestUseCases.ts |
Converted to factory function with DI dependencies |
src/shared/guest/store/guestStore.ts |
New guest mode signal store |
src/modules/user/application/usecases/userUseCases.ts |
Converted from singleton to factory function |
src/modules/weight/application/weight/usecases/weightUseCases.ts |
Module-level onMount/createEffect calls that won't execute properly |
src/sections/*/components/*.tsx |
Multiple components updated to use DI container with inconsistent patterns (some cache at component level, others call inline) |
src/modules/*/application/*.ts |
Repository selection logic updated to use DI container |
.github/copilot-instructions.md |
Refined instructions with less prescriptive guidance and better solo project adaptations |
.zed/settings.json, .zed/debug.json |
Added tab size, non-relative import preferences, and debug configuration |
.vscode/launch.json |
Updated port from 8080 to 3000 |
tsconfig.json |
Formatting-only changes (array elements on separate lines) |
Comments suppressed due to low confidence (1)
src/sections/common/components/buttons/RemoveFromRecentButton.test.tsx:91
- The test removed the mock for
authUseCases.currentUserIdOrGuestId()(previously on line 88) but didn't add mocking for the new DI container pattern. SinceRemoveFromRecentButtonnow callsuseCases.authUseCases(), the tests need to mock the DI container.
Add proper mocking for the DI container:
vi.mock('~/di/useCases', () => ({
useCases: {
authUseCases: vi.fn(() => ({
currentUserIdOrGuestId: vi.fn(() => mockUserId),
})),
},
}))| createEffect(() => { | ||
| const isGuest = authUseCases.currentUserIdOrGuestId() === GUEST_USER_ID | ||
| guestStore.setGuestModeEnabled(isGuest) | ||
| }) |
Copilot
AI
Dec 5, 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.
createEffect is called outside a reactive root, which will cause it to not run. Unlike createAuthDI, this function doesn't wrap its logic in createRoot.
The effect should run inside a reactive root to establish reactivity:
export function createGuestDI(di: GuestDI) {
return createRoot(() => {
const guestStore = createGuestStore()
const authUseCases = di.authUseCases()
createEffect(() => {
const isGuest = authUseCases.currentUserIdOrGuestId() === GUEST_USER_ID
guestStore.setGuestModeEnabled(isGuest)
})
return { guestStore, authUseCases }
})
}| } | ||
|
|
||
| try { | ||
| const authUseCases = useCases.authUseCases() |
Copilot
AI
Dec 5, 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.
Calling useCases.authUseCases() inside the async handler means the use cases are recreated on every conversion. This is inefficient and inconsistent with the pattern used in other components.
Move this to the component level:
export function ItemChildrenEditor(props: ItemChildrenEditorProps) {
const authUseCases = useCases.authUseCases()
const handleConvertToRecipe = async () => {
// ... validation
const userId = authUseCases.currentUserIdOrGuestId()
// ... rest of handler
}
}
Remove outdated memory-related instructions from Copilot guidance and refine the overall instructions for clarity. Update VSCode launch configuration to use localhost:3000. Introduce a new debug configuration file and add a tab size setting. Implement non-relative imports for TypeScript and restructure the dependency injection across various modules, improving the initialization and selection processes. Enhance UI accessibility by adding button type attributes and titles.
Fix #515