Skip to content

refactor(arch): extract TemplateEngine with single-pass registry (#211) - #217

Merged
pajoma merged 1 commit into
developfrom
feat/211-template-engine-dry
May 18, 2026
Merged

refactor(arch): extract TemplateEngine with single-pass registry (#211)#217
pajoma merged 1 commit into
developfrom
feat/211-template-engine-dry

Conversation

@pajoma

@pajoma pajoma commented May 18, 2026

Copy link
Copy Markdown
Owner

Summary

  • Extracts resolveDate and toMomentFormat into src/journal/template-engine.ts with a central TEMPLATE_VARIABLE_MAP registry — adding a new template variable now requires editing exactly one place
  • Deletes both parallel switch/case blocks from src/util/dates.ts and the third duplicate from src/test/direct/path-parse-with-date.ts
  • Fixes the ${week} gap in toMomentFormat (was silently dropped; now maps to 'w')
  • Thread-safe locale handling: moment(date).locale(loc) replaces global moment.locale() mutation
  • Single-pass regex.replace(callback) replaces multiple forEach + string.replace calls

Closes

Closes #211

Spec / Plan

Test plan

  • src/test/suite/template-engine.test.ts — 21 new assertions covering all 7 named variables, custom ${d:fmt}, locale isolation, toMomentFormat mappings including ${week}, edge cases
  • Existing replace-variables-in-string.ts and path-parse-with-date.ts direct tests updated to import from new engine — behavior unchanged
  • npx tsc --noEmit passes clean
  • npm run compile (esbuild) passes clean
  • All 21 behavior assertions verified via direct node execution

🤖 Generated with Claude Code

Replace parallel switch/case blocks in util/dates.ts with a central
TEMPLATE_VARIABLE_MAP in src/journal/template-engine.ts. Each entry
holds both a resolve fn and a momentFormat string, making it
structurally impossible for the two resolution modes to drift.

- resolveDate: thread-safe via moment(date).locale(loc), single-pass regex callback
- toMomentFormat: same registry, also fixes the ${week} → 'w' gap
- Deletes third duplicate from src/test/direct/path-parse-with-date.ts
- Updates all callers: paths.ts, conf.ts, sync-daily-links.ts

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@pajoma

pajoma commented May 18, 2026

Copy link
Copy Markdown
Owner Author

Architecture Review: Final Approval (PR #217)

I have performed a critical architectural review of this PR. The implementation is exceptionally clean and rigorously adheres to the mandates set during the planning phase.

Key Achievements:

  • Definitive DRY: Successfully consolidated three parallel, drifting implementations into a single, high-performance registry. This is the definitive fix for the Knowledge Duplication risk identified in our audit.
  • Structural Consistency: The registry pattern ensures that momentFormat and actual resolve logic can never drift apart again.
  • Bug Resolution: Correctly identified and fixed the missing ${week} mapping in the path-preview logic, closing a long-standing functional gap.
  • Safety & Performance: The transition to isolated locales (moment().locale()) and single-pass regex replacement provides both thread-safety and (N)$ efficiency.
  • Test Integrity: The comprehensive new unit test suite, particularly the locale isolation tests, provides the high-confidence verification needed for core domain services.

This refactor significantly raises the architectural standard of the project. LGTM.


Note: Gemini was involved in creating this comment (but not without consulting me).

@pajoma
pajoma merged commit 44db519 into develop May 18, 2026
2 checks passed
@pajoma
pajoma deleted the feat/211-template-engine-dry branch May 18, 2026 07:09
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.

Arch: Consolidate Template and Date replacement logic (DRY)

1 participant