-
Notifications
You must be signed in to change notification settings - Fork 0
Description
Problem
Every message handler in src/background/handlers/ is called with four positional arguments:
// messageRouter.js
handler(request, dependencies, sendResponse, finishRequest)Of the 7 handler files, only 2 use dependencies at all (sessionHandlers.js and onboardingHandlers.js, 5 specific functions). Every other handler either ignores it silently or marks it _dependencies. Several handlers also ignore _request. This means 30+ handler functions carry parameters purely for positional reasons, not because they need them.
The root issue is positional coupling — all handlers must accept all four args in the correct order even if they only need one or two.
Proposed Fix — Option 1: Context Object
Replace the positional call in messageRouter.js with a single context object:
// messageRouter.js — before
handler(request, dependencies, sendResponse, finishRequest)
// messageRouter.js — after
handler({ request, sendResponse, finishRequest })Each handler destructures only what it needs:
// Handler that needs everything
export function handleGetOrCreateSession({ request, sendResponse, finishRequest }) { ... }
// Handler that only needs two things — no unused params at all
export function handleGetAllProblems({ sendResponse, finishRequest }) { ... }
// Handler that needs nothing from request
export function handleProblemSubmitted({ sendResponse, finishRequest }) { ... }The 5 handlers in sessionHandlers and onboardingHandlers that currently pull from dependencies would instead import withTimeout, cleanupStalledSessions, checkOnboardingStatus, and completeOnboarding directly — these are all stable exported functions, not runtime values.
Why This Pattern
request, sendResponse, and finishRequest are runtime values that arrive per-message from Chrome's listener — they must be passed through. A context object is the right vehicle for them because:
- No positional ordering bugs (
sendResponseandfinishRequestare easy to transpose silently) - Handlers declare only what they actually use — no
_noise - Adding new context (e.g.
sender,tabId) requires no signature changes in handlers that don't need it
This pattern is already used elsewhere in the codebase — useFloatingHintHandlers.js and several hook files use clean object destructuring for exactly this reason.
Files Affected
| File | Unused param instances |
|---|---|
src/background/handlers/problemHandlers.js |
8 handlers |
src/background/handlers/dashboardHandlers.js |
~14 handlers |
src/background/handlers/storageHandlers.js |
~15 handlers |
src/background/handlers/strategyHandlers.js |
6 handlers |
src/background/handlers/hintHandlers.js |
4 handlers |
src/background/handlers/sessionHandlers.js |
~10 handlers (3 actually use deps) |
src/background/handlers/onboardingHandlers.js |
~8 handlers (2 actually use deps) |
src/background/messageRouter.js |
call site |
Other Areas That Could Benefit
The same pattern could clean up unused parameters in:
src/background/messageRouter.jsswitch cases — several inline handlers repeat the same pattern- React component callback props — any
(value, _index, _array) =>style callbacks in JSX files where only the first arg is used - Service functions with option bags — any service method taking 3+ positional config args could use an options object
Notes
withTimeoutandcleanupStalledSessionsare exported frombackgroundHelpers.jsand can be imported directlycheckOnboardingStatusandcompleteOnboardingare already imported inbackground/index.jsfromonboardingService.js- The
sessionHandlerstests mockwithTimeoutviadeps— these would need updating to usejest.mockafter the refactor