|
| 1 | +# Plan: Consolidate Template and Date Replacement Logic — #211 |
| 2 | + |
| 3 | +## Reference spec |
| 4 | +[docs/specs/2026-05-18-211-template-engine-dry.md](../specs/2026-05-18-211-template-engine-dry.md) |
| 5 | + |
| 6 | +## Approved decisions |
| 7 | +- Module: `src/journal/template-engine.ts` (journal domain layer, not util) |
| 8 | +- `${week}` gap: fix in scope — add to `toMomentFormat` as well |
| 9 | +- Thread-safe locales: `moment(date).locale(loc)` — no global `moment.locale()` calls |
| 10 | +- Single-pass: `template.replace(regex, callback)` — no forEach + multiple replaces |
| 11 | +- Registry pattern: each entry holds both `resolve` fn and `momentFormat` string |
| 12 | + |
| 13 | +## Approach |
| 14 | +Replace two parallel switch/case blocks with a single `TEMPLATE_VARIABLE_MAP` registry. Both `resolveDate` and `toMomentFormat` use one regex callback pass over the registry. Structural impossibility of drift: adding a variable requires one map entry with both fields. |
| 15 | + |
| 16 | +Trade-off: moving functions from `util/` to `journal/` means callers in `vscode/` and `features/` import cross-layer. Acceptable because `util/` was a generic dumping ground; the domain layer is the right home. The barrel (`src/journal/index.ts`) re-exports the new functions, keeping import paths short. |
| 17 | + |
| 18 | +## Steps |
| 19 | + |
| 20 | +### 1 — Create `src/journal/template-engine.ts` |
| 21 | +New file. Contains: |
| 22 | +- `TEMPLATE_VARIABLE_MAP` (module-private) — 7 named entries each with `momentFormat` and `resolve(m: Moment): string` |
| 23 | +- `TEMPLATE_VAR_REGEX` (module-private) — same pattern as current `regExpDateFormats` in `dates.ts` |
| 24 | +- `resolveDate(template, date, locale?)` — single-pass regex replace using `moment(date).locale(locale)`, not global locale |
| 25 | +- `toMomentFormat(template)` — single-pass regex replace returning moment format tokens; `${d:fmt}` → `fmt` passthrough; `${week}` → `'w'` (fixing the existing gap) |
| 26 | + |
| 27 | +Why first: all subsequent steps depend on this file existing. |
| 28 | + |
| 29 | +### 2 — Update `src/util/dates.ts` |
| 30 | +Remove: |
| 31 | +- `regExpDateFormats` constant |
| 32 | +- `replaceDateFormats` function |
| 33 | +- `replaceDateTemplatesWithMomentsFormats` function |
| 34 | + |
| 35 | +Keep all other functions (`formatDate`, `getCurrentISOWeek`, `getISOWeekYear`, `getDatesOfISOWeek`, `getDayOfWeekForString`, `getMonthForString`). |
| 36 | + |
| 37 | +Why: eliminates the source of duplication. Must happen after step 1. |
| 38 | + |
| 39 | +### 3 — Update `src/util/index.ts` |
| 40 | +Remove `replaceDateFormats` and `replaceDateTemplatesWithMomentsFormats` from the `export { ... } from './dates'` block. |
| 41 | + |
| 42 | +Why: barrel must not re-export deleted symbols. |
| 43 | + |
| 44 | +### 4 — Update `src/journal/index.ts` |
| 45 | +Add exports: |
| 46 | +```typescript |
| 47 | +export { resolveDate, toMomentFormat } from './template-engine'; |
| 48 | +``` |
| 49 | +Why: makes the new API reachable as `J.Journal.resolveDate` and via named imports from `'../journal'` for vscode/features layers. |
| 50 | + |
| 51 | +### 5 — Update `src/journal/paths.ts` |
| 52 | +Line 26: change `import { replaceDateTemplatesWithMomentsFormats } from '../util/dates'` to `import { toMomentFormat } from './template-engine'`. |
| 53 | + |
| 54 | +Lines 78–79: replace `replaceDateTemplatesWithMomentsFormats(...)` → `toMomentFormat(...)`. |
| 55 | + |
| 56 | +Why: closest caller; direct sibling import. |
| 57 | + |
| 58 | +### 6 — Update `src/vscode/conf.ts` |
| 59 | +Line 26: change `import { replaceDateFormats, replaceVariableValue } from '../util'` — remove `replaceDateFormats`, add `import { resolveDate } from '../journal/template-engine'`. |
| 60 | + |
| 61 | +Lines 342, 368: replace `replaceDateFormats(...)` → `resolveDate(...)`. |
| 62 | + |
| 63 | +Why: conf.ts is in the vscode layer; import from journal domain is fine. |
| 64 | + |
| 65 | +### 7 — Update `src/features/sync/sync-daily-links.ts` |
| 66 | +Line 4: change `import { getDatesOfISOWeek, replaceDateFormats, replaceVariableValue } from '../../util'` — remove `replaceDateFormats`, add `import { resolveDate } from '../../journal/template-engine'`. |
| 67 | + |
| 68 | +Line 80: replace `replaceDateFormats(...)` → `resolveDate(...)`. |
| 69 | + |
| 70 | +Why: same locale-aware signature; drop-in replacement. |
| 71 | + |
| 72 | +### 8 — Fix `src/test/direct/path-parse-with-date.ts` |
| 73 | +Remove local duplicate `replaceDateTemplatesWithMomentsFormats` function (lines 124+). |
| 74 | +Replace its two call sites (lines 94–95) with `toMomentFormat` imported from `../../journal/template-engine`. |
| 75 | + |
| 76 | +Why: this is the third copy of the function — the most egregious duplication site. |
| 77 | + |
| 78 | +### 9 — Fix `src/test/direct/replace-variables-in-string.ts` |
| 79 | +Update import: `import { replaceDateFormats } from "../../util/dates"` → `import { resolveDate } from "../../journal/template-engine"`. |
| 80 | +Update call sites: `replaceDateFormats(...)` → `resolveDate(...)`. |
| 81 | + |
| 82 | +Why: test file must import from the new location; function signature is identical. |
| 83 | + |
| 84 | +## Test scenarios |
| 85 | + |
| 86 | +**Unit (new file `src/test/suite/template-engine.test.ts`):** |
| 87 | +- `resolveDate-named-vars`: `${year}`, `${month}`, `${day}`, `${localTime}`, `${localDate}`, `${weekday}`, `${week}` each replaced with correct formatted value for a known date |
| 88 | +- `resolveDate-custom-format`: `${d:YY}` → two-digit year; `${d:dddd}` → weekday name |
| 89 | +- `resolveDate-no-vars`: template without `${...}` returns unchanged |
| 90 | +- `resolveDate-locale-isolation`: calling `resolveDate` twice with different locales returns locale-specific results without affecting a subsequent call (proves no global locale mutation) |
| 91 | +- `toMomentFormat-named-vars`: all 7 variables replaced with correct moment tokens, including `${week}` → `'w'` |
| 92 | +- `toMomentFormat-custom-format`: `${d:YY}` → `'YY'` (passthrough) |
| 93 | +- `toMomentFormat-week-fixed`: `${week}` → `'w'` (explicit test for the closed gap) |
| 94 | + |
| 95 | +**Integration (existing tests, unchanged assertions):** |
| 96 | +- `replace-variables-in-string.ts` — same outputs as before (behavior parity) |
| 97 | +- `path-parse-with-date.ts` — path construction still produces same paths after removing local duplicate |
| 98 | + |
| 99 | +## Dependencies |
| 100 | +- feat-210 (#210) must land on `develop` before this PR is merged (avoid double-churn on `src/model/`). Implementation can proceed on a feature branch in parallel; merge order matters. |
| 101 | + |
| 102 | +## Risk |
| 103 | +- **Locale regression:** old code mutated global `moment.locale(locale)` before formatting. New code uses `moment(date).locale(loc)`. Behavior is equivalent for sequential calls but safe for concurrent calls. The `resolveDate-locale-isolation` test scenario covers this. |
| 104 | +- **`${week}` toMomentFormat gap fixed:** `toMomentFormat` previously silently dropped `${week}`. After this PR it maps to `'w'`. Any caller that depended on the silent-drop behavior (passing output to `moment.format`) would change. Audit: only caller is `src/journal/paths.ts` lines 78–79. Those paths do not use `${week}`, so no behavior change in practice. |
| 105 | + |
| 106 | +## Rollback |
| 107 | +Revert the feature branch. No schema migrations, no config changes, no data touched. |
| 108 | + |
| 109 | +## Implementation branch |
| 110 | +`feat/211-template-engine-dry` off `develop` (or off the feat-210 branch if sequencing is needed). |
0 commit comments