Skip to content

refactor: Ctrl service-locator → app/Container DI (Phase 2, #234) - #236

Merged
pajoma merged 1 commit into
developfrom
refactor/234-phase2-di-container
Jun 2, 2026
Merged

refactor: Ctrl service-locator → app/Container DI (Phase 2, #234)#236
pajoma merged 1 commit into
developfrom
refactor/234-phase2-di-container

Conversation

@pajoma

@pajoma pajoma commented Jun 2, 2026

Copy link
Copy Markdown
Owner

Phase 2 of #234 — replace the Ctrl service-locator with a real composition root + dependency injection.

Depends on #235 (Phase 1) being merged.

Changes

  • src/app/ new composition root:
    • container.tsContainer implements JournalController. Single-pass constructor (configSource, loggerFactory) builds the whole service graph in one go. Removes the two-phase initServices + ! non-null fields.
    • register.ts — command / code-action / cache-invalidation registration.
    • startup.ts — extension lifecycle (moved from vscode/startup.ts), .then() chain rewritten to async/await.
  • Commands, UI providers and features now depend on the JournalController interface, never the concrete controller class.
  • Expanded IConfiguration with the 11 methods consumers actually call (getNavigationMode, getScopeDefinitions, getWeeksPathPatternRaw, getWeeksFilePatternRaw, getWeeklySyncConfig, getTimeStringTemplate, getFileLinkInlineTemplate, getDailyLinkInlineTemplate, getBasePathForLocalOpen, getResolvedEntryPathForLocalOpen, getWeekPathPatternForLocalOpen).
  • Moved WeeklySyncConfig type to model/. journal/paths.ts now types its config param as IConfiguration (drops a journal → vscode coupling).
  • Deleted src/util/controller.ts and src/vscode/startup.ts.
  • Tests construct new Container(cfg, () => logger) instead of new Ctrl(cfg); ctrl.initServices(logger).

Lifecycle decision (per #234 review)

Container is the single owner; all services are singletons (one instance per activation), matching the old single-Ctrl model. No separate state module.

Verification

  • npm run compile-tests (tsc) ✔
  • npm run compile (esbuild) ✔
  • npm run lint
  • Full suite: 238 passing, exit 0

Acceptance (#234)

  • No command/provider depends on concrete Ctrl/Container — all take JournalController.
  • Single wiring point in app/container.ts.
  • Two-phase init removed.

Refs #234.

🤖 Generated with Claude Code

Phase 2 of #234. Introduce src/app/ composition root: Container
(implements JournalController, single-pass ctor with a logger factory —
removes two-phase initServices/!), register.ts (command/provider wiring),
startup.ts (lifecycle, moved from vscode/). Consumers now depend on the
JournalController interface, not the concrete controller class.

Expand IConfiguration with the methods consumers actually call
(getNavigationMode, getScopeDefinitions, getWeek*PatternRaw, *ForLocalOpen,
getWeeklySyncConfig, getTimeStringTemplate, get*LinkInlineTemplate). Move
WeeklySyncConfig type to model. paths.ts now types config as IConfiguration.

Delete src/util/controller.ts and src/vscode/startup.ts.

Refs #234.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@pajoma

pajoma commented Jun 2, 2026

Copy link
Copy Markdown
Owner Author

Brooks-Lint PR Review: Approved

Health Score: 92/100

Phase 2 of #234 is successfully implemented. This PR replaces the \Ctrl\ service-locator with a dedicated composition root in \�pp/container.ts, significantly improving the system's modularity and testability.

Key Findings:

  • Inversion of Control: The service graph is now constructed in a single pass. Commands depend on the \JournalController\ interface, enabling true dependency injection.
  • Robustness: The removal of two-phase init (\initServices) eliminates fragile non-null assertions across the controller.
  • Configuration: While \IConfiguration\ remains large, this is a known transitional state to be addressed in Phase 3.
  • Verification: 238 tests passed; zero regressions in behavior.

Recommendation: Merge immediately to unblock Phase 3 (Split Configuration).

@pajoma
pajoma merged commit 52e1258 into develop Jun 2, 2026
2 checks passed
@pajoma
pajoma deleted the refactor/234-phase2-di-container branch June 2, 2026 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant