Skip to content

Commit b74c324

Browse files
pajomaclaude
andcommitted
docs: spec+plan for package-by-feature restructure
Refs #234. Target structure, 5-phase migration, separation rules, shared/events for cross-feature comms, singleton DI lifecycle. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent ff8d9f0 commit b74c324

2 files changed

Lines changed: 175 additions & 0 deletions

File tree

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
# Plan — Issue #234: Restructure `src/` to package-by-feature + DI
2+
3+
> **Spec:** [docs/specs/2026-06-02-234-package-by-feature.md](../specs/2026-06-02-234-package-by-feature.md)
4+
> **Issue:** [pajoma/vscode-journal#234](https://github.com/pajoma/vscode-journal/issues/234)
5+
> **Branch:** TBD (`refactor/234-package-by-feature`, off `develop` after preconditions met)
6+
> **Created:** 2026-06-02
7+
8+
## Approach
9+
10+
Five sequential phases, each independently shippable and test-green. Order minimizes blast radius: untangle dependencies before moving files, move files before changing semantics. Phase 5 (vscode-free domain) is the only behavior-adjacent step and may split into a follow-up issue.
11+
12+
## Preconditions
13+
14+
- Merge open PRs (#231, #233) before starting Phase 1 — it touches nearly every file. Do not start over an open PR.
15+
- `rm -rf out/` before first compile to drop stale `.test.js` (per `AGENTS.md`).
16+
17+
## Phases
18+
19+
### Phase 1 — Kill the `J` barrel
20+
- Replace every `import * as J from '..'` + `J.X.Y` with named imports from the concrete module.
21+
- Reduce `src/index.ts` to type-only re-exports or remove it.
22+
- Pure mechanical; zero behavior change. Largest diff.
23+
- Verify: `npm run check` green; `grep -r "import \* as J" src/` empty.
24+
25+
### Phase 2 — `Ctrl` → DI container
26+
- Add `app/container.ts` owning singleton service construction (the current `initServices` graph).
27+
- Add `app/register.ts` for command/provider registration; move syntax-highlight + cache wiring to `app/startup.ts`.
28+
- Change command/provider constructors to take narrow interfaces (`IReader`, `IWriter`, `IConfiguration`, …) instead of `Ctrl`.
29+
- Remove two-phase `!` init; container builds in one pass after logger exists.
30+
- Verify: no constructor references concrete `Ctrl`; suite green.
31+
32+
### Phase 3 — Split `Configuration`
33+
- `SettingsReader` — raw `journal.*` reads + scope/windows normalization.
34+
- `PathResolver` — entry/notes/weekly path + file patterns (local + remote variants).
35+
- `TemplateProvider` — header/inline/time template lookups (absorbs `vscode/template-service.ts`).
36+
- Keep `IConfiguration` as the composed facade if needed to limit call-site churn; each class ≤~300 lines.
37+
- Verify: `read-templates.test.ts`, `template-engine.test.ts`, path tests green.
38+
39+
### Phase 4 — Move files into feature folders
40+
- `git mv` into `features/{entries,notes,weekly,tasks,navigation,smart-input,tools}` and `shared/{config,fs,logging,dates,strings,templates,model}`.
41+
- Add `shared/events/` typed `EventEmitter`; convert the `reader.onNotesInjected` callback and the inline weekly-sync in `show-entry-for-date.ts` to published events (`entryOpened`), subscribed by `weekly`/`notes`.
42+
- Fix imports. No logic change beyond the event rewiring.
43+
- Verify: no cross-feature internal imports (grep `features/*/` for sibling-feature paths); suite green.
44+
45+
### Phase 5 — vscode-free domain (may split to follow-up)
46+
- Introduce `shared/editor/` adapter (open/show/save `TextDocument`).
47+
- Change `domain/` (reader/writer/inject) to return paths/data, not `TextDocument`; commands call the editor adapter.
48+
- Verify: `grep -r "vscode" src/features/*/domain/` empty; add pure-node unit tests for domain logic.
49+
50+
## Test scenarios
51+
52+
| Scenario | Check |
53+
|----------|-------|
54+
| Every phase | `npm run check` green |
55+
| Phase 1 done | `grep -r "import \* as J" src/` → empty |
56+
| Phase 2 done | no command ctor takes `Ctrl`; `app/container.ts` sole wiring point |
57+
| Phase 3 done | `Configuration` → 3 classes, each ≤~300 lines; template/path tests green |
58+
| Phase 4 done | no `features/<a>` imports `features/<b>` internals; events flow via `shared/events/` |
59+
| Phase 5 done | `src/features/*/domain/` has zero `vscode` imports; new node-only domain tests pass |
60+
| Regression | full suite (commands-entry, commands-note, commands-weekly, scan-entries, sync) green after each phase |
61+
62+
## Dependencies
63+
64+
- Blocked by open PRs #231, #233 (merge first).
65+
- Touches `docs/PLAN.md` (update Phase 2+ to reflect new layout).
66+
67+
## Risk
68+
69+
| Risk | Mitigation |
70+
|------|-----------|
71+
| Phase 1 huge diff → rebase hell | Merge pending PRs first; land Phase 1 fast as its own PR |
72+
| Hidden cycles surface during barrel removal | Fix per-module; named imports make cycles explicit |
73+
| `Configuration` split breaks scope/remote path resolution | Phase 3 keeps `IConfiguration` facade; lean on existing path/template tests |
74+
| Phase 5 changes return types broadly | Split to its own issue/PR; ship Phases 1–4 first |
75+
| Event rewiring changes fire-and-forget timing | Keep events fire-and-forget with same error-swallowing as today; assert via existing sync tests |
76+
77+
## Rollback
78+
79+
Each phase is a separate PR. Revert the offending PR; earlier phases remain valid. No settings/schema migration, so no data rollback.
80+
81+
## Reference spec
82+
83+
[docs/specs/2026-06-02-234-package-by-feature.md](../specs/2026-06-02-234-package-by-feature.md)
Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
# Spec: Restructure `src/` to package-by-feature with shared kernel + DI (#234)
2+
3+
## Goal
4+
5+
Move `src/` from package-by-layer (`commands/`, `journal/`, `model/`, `vscode/`, `features/`, `ui/`, `util/`) to **package-by-feature** with a thin, dependency-free shared kernel. Each feature lives in one directory. Replace the `Ctrl` service-locator with constructor dependency injection, kill the `J` namespace barrel, and (last) make the domain layer VS Code-free.
6+
7+
No user-facing behavior change.
8+
9+
## Why now
10+
11+
A single capability is smeared across many directories. "Notes" spans `commands/show-note.ts`, `journal/parser.ts`, `journal/writer.ts`, `features/entries/load-note.ts`, `features/sync/sync-note-links.ts`, and `vscode/conf.ts` — six directories for one feature. The layering also hides real dependencies and blocks unit testing. This aligns with `docs/PLAN.md` Phase 2+ (DI, named imports, native `async`/`await`, `vscode.workspace.fs`).
12+
13+
## Problems addressed
14+
15+
1. **`J` namespace barrel = circular god-import.** `src/index.ts` re-exports every submodule as `J.Util`, `J.Journal`, etc. Files do `import * as J from '..'` then `J.Util.Ctrl`. Those files are themselves re-exported by `index.ts`, so `index.ts → commands/show-note.ts → index.ts` is a cycle. Kills tree-shaking, breaks go-to-definition, hides dependencies.
16+
2. **`Ctrl` service-locator god-object** (`util/controller.ts`). Every command/provider receives the full `Ctrl` and reaches `ctrl.reader`/`ctrl.writer`/`ctrl.ui`/`ctrl.config`/`ctrl.inject`. Narrow interfaces (`IReader`, …) exist in `model/interfaces.ts` but are unused at call sites. Two-phase init forces `!` assertions on every field. The controller also lives in `util/` — wrong home.
17+
3. **Domain coupled to `vscode`.** `Reader`/`Writer`/`Inject` import `vscode` and return `vscode.TextDocument`, so core logic can't be unit-tested without the Extension Host (hence the no-display workaround in `AGENTS.md`).
18+
4. **`Configuration` god-class** — 708 lines (`vscode/conf.ts`), ~40 public methods mixing raw setting reads, path-pattern resolution, template lookup, scope resolution, windows-path normalization, and local-vs-remote path variants.
19+
5. **Inconsistent buckets.** No rule separates `journal/` vs `features/` vs `ui/`. `scan-entries` is a "feature" but `reader` is "journal"; `codeactions` is in `ui` but `sync` is in `features`; two template modules split across layers (`journal/template-engine.ts` vs `vscode/template-service.ts`).
20+
6. **Orchestration leaks into commands.** `show-entry-for-date.ts` `loadPageForInput` runs fire-and-forget weekly sync via nested `.then/.catch`.
21+
7. **Mixed async styles**`.then()` chains (`startup.run`, `loadPageForInput`) alongside `async/await`.
22+
23+
## Target structure
24+
25+
```
26+
src/
27+
extension.ts # activate/deactivate only
28+
app/ # composition root (was vscode/startup + util/controller)
29+
container.ts # builds the dependency graph — replaces Ctrl service-locator
30+
register.ts # command/provider registration
31+
startup.ts # syntax highlighting, cache-invalidation wiring
32+
shared/ # kernel — NO feature dependencies
33+
config/ # split Configuration: SettingsReader | PathResolver | TemplateProvider
34+
fs/ # IFileSystem + VscodeFileSystem
35+
logging/ dates/ strings/
36+
templates/ # merge template-engine + template-service
37+
editor/ # the single vscode-TextDocument adapter (open/show/save)
38+
events/ # typed EventEmitter / mediator for cross-feature signals
39+
model/ # shared types: Input, FileEntry, ScopedTemplate
40+
features/
41+
entries/ # today | tomorrow | yesterday | date | input + entry reader/writer
42+
notes/ # show-note + load-note + note-path + sync-note-links
43+
weekly/ # weekly entry + watcher + sync-daily-links
44+
tasks/ # copy/shift + codeactions + migrate codelens
45+
navigation/ # prev/next entry
46+
smart-input/ # match-input + parser + input dialog
47+
tools/ # print-time | print-duration | print-sum
48+
```
49+
50+
Each feature folder: `commands/` (VS Code handlers) · `domain/` (pure logic, no vscode) · `ui/` (providers).
51+
52+
### Separation rules
53+
54+
- A feature may import from `shared/`. A feature must **never** import another feature's internals.
55+
- Cross-feature interaction goes through `shared/events/` — a small typed `EventEmitter`/mediator. Example: `entries` emits `entryOpened``weekly` subscribes and runs daily-link sync. No feature-to-feature imports. *(Maintainer recommendation, #234 review.)*
56+
- `domain/` imports no `vscode`; it returns data/paths. `commands/` plus the `shared/editor/` adapter handle `TextDocument`.
57+
- Inject narrow interfaces (`IReader`, not `Ctrl`) via constructor. `app/container.ts` builds the graph once.
58+
59+
### Service lifecycle
60+
61+
`app/container.ts` is the single owner of service lifecycles. All services are **singletons** (one instance per activation), matching the current single-`Ctrl` model. No separate `shared/state/` module is introduced unless genuinely mutable cross-feature state appears — out of scope for this issue. *(Maintainer recommendation, #234 review.)*
62+
63+
## In scope
64+
65+
- Directory restructure to package-by-feature.
66+
- Remove `J` barrel / `import * as J`; named imports throughout.
67+
- Replace `Ctrl` with `app/container.ts` DI; commands take narrow interfaces.
68+
- Split `Configuration` into ≤3 focused classes.
69+
- Add `shared/events/` typed emitter for cross-feature signals.
70+
- Make `domain/` VS Code-free via `shared/editor/` adapter (Phase 5 — may split to a follow-up issue).
71+
- Update `docs/PLAN.md` to reflect the new structure.
72+
73+
## Out of scope
74+
75+
- No user-facing behavior change. No new settings, commands, or templates.
76+
- No change to the `journal.*` settings schema or file/path patterns in `package.json`.
77+
- i18n strings unchanged (English-only, per `AGENTS.md`).
78+
- No new mutable state module (`shared/state/`) unless a concrete need surfaces.
79+
80+
## Preconditions
81+
82+
- **Merge pending PRs first** (notably #231, #233). Phase 1 touches nearly every file; landing it over open PRs causes large rebase conflicts. *(Maintainer recommendation, #234 review.)*
83+
84+
## Acceptance criteria
85+
86+
1. `npm run check` (lint + compile + test) green after **every** phase.
87+
2. No `import * as J from` remaining; `src/index.ts` barrel removed or reduced to type-only re-exports.
88+
3. No command/provider depends on concrete `Ctrl`; all take narrow interfaces wired in `app/container.ts`.
89+
4. `Configuration` split into ≤3 focused classes, none over ~300 lines.
90+
5. Each feature directory is self-contained; no cross-feature internal imports (verifiable by grep/lint rule).
91+
6. Cross-feature signals flow through `shared/events/`.
92+
7. `docs/PLAN.md` updated to reflect the new structure.

0 commit comments

Comments
 (0)