refactor: declarative persistence + split store/main god-modules - #211
Merged
Merged
Conversation
…sistence binding The nine StoreController subclasses plus their abstract base were ~215 lines of near-identical boilerplate: each subscribed a store, wrote its value into the matching plugin.settings field, and called saveSettings(). main.ts then declared nine controller fields, instantiated nine .on()s, and .off()'d nine in onunload. Collapse all of it into src/store/persistence.ts: a single declarative BINDINGS table (store -> settings field, with the few real exceptions — forcing built-in playlist names, skipping a no-op primitive write) and one bindStoresToSettings() that subscribes them all and returns a single unsubscriber. main.ts now holds one storeUnsubscribers list disposed in onunload. QueueController also removed the now-playing episode from the queue on currentEpisode change. That is queue automation, not persistence, so it moves to subscribeQueueToCurrentEpisode() next to the other queue automation in the store module, with its own unit test. Behavior-preserving: settings still persist identically (verified live — data.json round-trips and the queue keeps its canonical name). No public API change.
…ules src/store/index.ts had grown to ~830 lines mixing playback state, the episode status/queue core, the offline downloads store, and the whole Latest Episodes aggregation engine (a derived store plus seven helpers). Pull the two self-contained concerns into their own files: - src/store/feeds.ts: savedFeeds, episodeCache, episodeListLimit, sanitizeEpisodeListLimit, and the latestEpisodes projection + helpers. - src/store/downloads.ts: the downloadedEpisodes store. Both are leaf modules — they import only types/constants, never the rest of the store layer — so there are no import cycles. index.ts re-exports them so `src/store` stays the single import surface; no consumer import changed. The inherently coupled playback/queue/episode-status core stays together in index.ts (down to ~510 lines), since splitting it would force currentEpisode<->queue<->playedEpisodes cycles. Pure code move, behavior-preserving. Gates green; verified live (plugin reloads with the new bundle, view intact, no console errors).
onload registered all 19 commands inline, making the plugin entry point ~790 lines and burying lifecycle wiring (stores, view, ribbon, protocol, media session) under a long wall of addCommand calls. Move every command into registerCommands(plugin) in src/commands.ts; onload now calls registerCommands(this) and drops to ~480 lines focused on lifecycle. The two helpers the commands need (captureTimestamp, getTranscriptionService) become public on the plugin — captureTimestamp is still also used by the media-session handler. Command order, ids, names, icons, and guard logic are unchanged. Behavior-preserving. Gates green; verified live (19 podnotes commands registered, Show PodNotes routes through the module and opens the view, no console errors).
Adversarial review flagged comments still naming LocalFilesController / "persistence controllers" / "stores to controllers" after the StoreController classes were replaced by the declarative persistence binding. Point them at the binding/ subscription model instead. Comment-only; no behavior change.
Deploying podnotes with
|
| Latest commit: |
d45208f
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://2c745eb6.podnotes.pages.dev |
| Branch Preview URL: | https://chhoumann-refactor-architect.podnotes.pages.dev |
chhoumann
marked this pull request as ready for review
June 21, 2026 17:39
chhoumann
added a commit
that referenced
this pull request
Jun 21, 2026
Second behavior-preserving architecture pass continuing #211. Net -103 LOC. - store purity: removeEpisode returns the file path; vault deletion left the store (kills the only production @ts-ignore) - templates: extract addEpisodeFileNameTags/legalizedNameTag (dedupe 4 engines) - transcripts: extract pure WAV chunking to audioChunker.ts (TranscriptionService 631->400; tests hit real exports, not copies) - settings: stackSettingVertically/renderMarkdownPreview helpers + OPML via pickFile - review fixes: per-caller pickFile cap/message; composed removeDownloadedEpisode (deleteEpisodeFile now private) Gates green (657 tests), live-verified in Obsidian, adversarially reviewed (PASS).
Contributor
|
🎉 This PR is included in version 2.17.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three behavior-preserving refactors of the store/persistence layer and the plugin
entry point, each separately gated and verified live in Obsidian. No user-facing
behavior change, no public API change, no settings/storage/URI change.
The goal was navigability and maintainability: collapse duplicated boilerplate,
break up the largest god-modules, and leave a clearer map of where things live.
1. Declarative settings persistence (
refactor(store))Nine near-identical
StoreControllersubclasses plus their abstract base (~215LOC) each subscribed a store, wrote its value into the matching
plugin.settingsfield, and called
saveSettings().main.tsdeclared nine controller fields,instantiated nine
.on()s, and.off()'d nine inonunload.Collapsed into
src/store/persistence.ts: one declarativeBINDINGStable(store -> settings field, with the few real exceptions — forcing the built-in
playlist names, the
hidePlayedEpisodesskip-if-unchanged guard) and a singlebindStoresToSettings(plugin)returning one unsubscriber.QueueControlleralsoremoved the now-playing episode from the queue on
currentEpisodechange; thatis queue automation, not persistence, so it moved to
subscribeQueueToCurrentEpisode()in the store module with its own unit test.2. Split the
store/index.tsgod-module (refactor(store))src/store/index.tshad grown to ~830 lines. Extracted the two self-containedconcerns into leaf modules (no sibling-store imports, so no import cycles):
src/store/feeds.ts— saved feeds, episode cache,episodeListLimit, and thelatestEpisodesaggregation engine + helpers.src/store/downloads.ts— thedownloadedEpisodesstore.index.tsre-exports both, sosrc/storestays the single import surface and noconsumer import changed. The inherently coupled playback/queue/episode-status core
stays together (~510 lines).
3. Extract command registration (
refactor(main))onloadregistered all 19 commands inline. Moved them toregisterCommands(plugin)insrc/commands.ts;onloaddropped from ~790 to~480 lines and now reads as lifecycle wiring (stores, view, ribbon, protocol,
media session). Command ids, names, icons, guards, and order are unchanged.
A follow-up commit fixes comments that still named the deleted controller classes.
Verification
npm run lint,format:check,typecheck,build,check:a11y, and the fullvitestsuite (655 tests) all pass.console errors, the view opens, all 19 commands register,
data.jsonround-trips,and the three built-in playlists keep their canonical names (Queue / Favorites /
Local Files) through the persistence binding.
minimalism). No correctness defects found; the one actioned finding (stale
comments) is fixed. Remaining notes are structural taste, recorded and consciously
deferred.
Risk / migration
None. Pure internal refactor — settings shape, persisted data, commands, and the
public API are byte-for-byte equivalent.