|
| 1 | +--- |
| 2 | +issue: "#201" |
| 3 | +date: 2026-05-18 |
| 4 | +slug: fix-201-circular-dep-controller |
| 5 | +spec: docs/specs/2026-05-17-fix-201-circular-dep-controller.md |
| 6 | +--- |
| 7 | + |
| 8 | +# Plan: Fix circular dependency in `src/util/controller.ts` (#201) |
| 9 | + |
| 10 | +## Reference spec |
| 11 | + |
| 12 | +[docs/specs/2026-05-17-fix-201-circular-dep-controller.md](../specs/2026-05-17-fix-201-circular-dep-controller.md) |
| 13 | + |
| 14 | +## Approach |
| 15 | + |
| 16 | +Single-file edit. The file already has direct imports for `Parser`, `Writer`, `Reader`, `Inject`, |
| 17 | +and `Dialogues` (added as part of earlier work). Three direct imports are still missing: |
| 18 | +`Configuration`, `Logger`, and `isNullOrUndefined`. Add those, remove the barrel import, then |
| 19 | +update the 15 `J.*` type annotations and one function call that still reference `J.VSCode.*`, |
| 20 | +`J.Journal.*`, and `J.Util.*`. |
| 21 | + |
| 22 | +No interface extraction, no injection changes — the approval comment noted those as |
| 23 | +long-term goals; this issue is scoped to breaking the barrel cycle only. |
| 24 | + |
| 25 | +## Steps |
| 26 | + |
| 27 | +1. **Add three missing direct imports** in `src/util/controller.ts`: |
| 28 | + - `import { Configuration } from '../vscode/conf'` |
| 29 | + - `import { Logger } from './logger'` |
| 30 | + - `import { isNullOrUndefined } from './util'` |
| 31 | + |
| 32 | +2. **Remove barrel import**: delete line `import * as J from '../.'`. |
| 33 | + |
| 34 | +3. **Update type annotations and usages** — all occurrences in the same file: |
| 35 | + |
| 36 | + | Old | New | Already imported? | |
| 37 | + |-----|-----|-------------------| |
| 38 | + | `J.VSCode.Configuration` | `Configuration` | After step 1 | |
| 39 | + | `J.VSCode.Dialogues` | `Dialogues` | Yes (line 30) | |
| 40 | + | `J.Journal.Parser` | `Parser` | Yes (line 26) | |
| 41 | + | `J.Journal.Writer` | `Writer` | Yes (line 27) | |
| 42 | + | `J.Journal.Reader` | `Reader` | Yes (line 28) | |
| 43 | + | `J.Journal.Inject` | `Inject` | Yes (line 29) | |
| 44 | + | `J.Util.Logger` | `Logger` | After step 1 | |
| 45 | + | `J.Util.isNullOrUndefined` | `isNullOrUndefined` | After step 1 | |
| 46 | + |
| 47 | + Affected locations in the file (15 references + 1 call): |
| 48 | + - Field declarations: `_config`, `_ui`, `_parser`, `_writer`, `_reader`, `_logger`, `_inject` |
| 49 | + - Constructor: `new J.VSCode.Configuration(...)` |
| 50 | + - `initServices`: `logger as J.Util.Logger` |
| 51 | + - Getter return types: `ui`, `writer`, `reader`, `parser`, `config`, `inject`, `logger` |
| 52 | + - Logger guard: `J.Util.isNullOrUndefined(this._logger)` |
| 53 | + |
| 54 | +4. **Run acceptance checks** locally: |
| 55 | + ``` |
| 56 | + npm run compile |
| 57 | + npm run lint |
| 58 | + npm test |
| 59 | + ``` |
| 60 | + |
| 61 | +5. **Open PR** against `develop`, referencing `#201`. |
| 62 | + |
| 63 | +## Test scenarios |
| 64 | + |
| 65 | +| Scenario | Type | Covers | |
| 66 | +|----------|------|--------| |
| 67 | +| **No barrel import** — grep `controller.ts` for `import \* as J` returns empty | static | AC 1 | |
| 68 | +| **No J. references** — grep `controller.ts` for `J\.` returns empty | static | AC 1 | |
| 69 | +| **Compile clean** — `npm run compile` exits 0 | build | AC 2 | |
| 70 | +| **Lint clean** — `npm run lint` exits 0 with no new warnings | lint | AC 4 | |
| 71 | +| **Full suite green** — `npm test` passes all existing tests | integration | AC 3 | |
| 72 | + |
| 73 | +No new unit tests required; this is a pure import-path change with no behavior delta. |
| 74 | + |
| 75 | +## Dependencies |
| 76 | + |
| 77 | +None. `controller.ts` already contains direct imports for 5 of the 8 classes (from prior work); |
| 78 | +only 3 new import lines are needed. |
| 79 | + |
| 80 | +## Risk |
| 81 | + |
| 82 | +Low. All types used in the updated annotations are already in scope via the direct imports added |
| 83 | +earlier (#212 or equivalent). The barrel import line is the only thing routing these names through |
| 84 | +`J.*`; removing it and redirecting the names is a compile-time-only change. |
| 85 | + |
| 86 | +ESLint `camelCase`/`PascalCase` rule: all new import names (`Configuration`, `Logger`, |
| 87 | +`isNullOrUndefined`) satisfy it. |
| 88 | + |
| 89 | +## Rollback |
| 90 | + |
| 91 | +Revert the single commit on the PR. No migration, no data change, no release artifact change. |
0 commit comments