scotttomaszewski_obsidian-d.../FOLLOWUP.md
Scott Tomaszewski c10c3d8902 Migrate file access from vault.adapter to Vault/FileManager APIs
Resolves FOLLOWUP.md #2. Replaces vault.adapter.* usage with the
intended Obsidian APIs:

- ESVApiService.saveESVApiResponseAsMdNote: Vault.create() for the note
  body + FileManager.processFrontMatter() for frontmatter; folder
  creation via getAbstractFileByPath/createFolder.
- DisciplesJournalPlugin.updateAllBibleNoteFrontmatter: reads canonical
  from metadataCache and updates via processFrontMatter.
- FrontmatterUtil: buildFrontmatterString/mergeCustomFrontmatterIntoExisting
  replaced by applyCustomFrontmatter, which mutates the frontmatter object
  directly (drops stringifyYaml round-tripping).
- BibleFiles.fileExistsForPassage uses getAbstractFileByPath (now sync);
  clearData deletes via FileManager.trashFile() to respect trash settings.
- All passage/content paths run through normalizePath().
- eslint config ignores .devbox/ tooling artifacts.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-05-31 08:53:07 -04:00

5 KiB
Raw Blame History

Follow-up work

This file tracks known issues that were intentionally deferred during the 0.13.0 standards/compliance pass. The codebase currently passes npx eslint . and npm run build with zero errors/warnings; the items below are silenced with scoped eslint-disable comments (each referencing this file) so the deferral is explicit rather than hidden.

Scope decision at the time: do the "safe subset" of standards fixes and leave the file-API internals and the event-handler internals functionally unchanged.


1. Hover-preview event listeners leak (highest priority) — RESOLVED

Files: src/core/BibleEventHandlers.ts, src/components/BibleReferenceRenderer.ts, src/components/BibleReferenceInlineExtension.ts, src/core/DisciplesJournalPlugin.ts

What was wrong: BibleEventHandlers was instantiated on every mouseover/mouseout, and each constructor added mousemove/click listeners to the global document that were only removed by a cleanup() that was never called (empty onunload), so listeners leaked for the app session. It also used bare document and setInterval, breaking pop-out windows.

Fix applied: BibleEventHandlers now extends Component and is a single, plugin-owned instance created in onload and registered with addChild(...). Its document listeners are attached via registerDomEvent (lazily per document, deduped through a WeakSet, so pop-out windows are tracked too) and the close-poll runs on a single registerInterval timer — all torn down automatically on unload. The hover callbacks reuse the shared instance instead of constructing new ones. The file-level eslint-disable banner (obsidianmd/prefer-active-doc, obsidianmd/prefer-window-timers, @typescript-eslint/no-deprecated) was removed.


2. File access via vault.adapter instead of the Vault/FileManager APIs — RESOLVED

Files: src/services/ESVApiService.ts, src/services/BibleFiles.ts, src/core/DisciplesJournalPlugin.ts

What was wrong (Obsidian guideline rules 1922):

  • vault.adapter.write(...) was used to create/overwrite notes (ESVApiService.saveESVApiResponseAsMdNote, DisciplesJournalPlugin.updateAllBibleNoteFrontmatter).
  • vault.adapter.exists / vault.adapter.mkdir were used for existence checks and directory creation.
  • BibleFiles.clearData used vault.adapter.rmdir(...), bypassing the user's trash settings.
  • Paths were built by string concatenation without normalizePath().

Fix applied:

  • ESVApiService.saveESVApiResponseAsMdNote now creates the note body with Vault.create() (reusing the existing TFile when present) and writes the API response + custom fields via FileManager.processFrontMatter(). Folder creation goes through Vault.getAbstractFileByPath() / Vault.createFolder().
  • DisciplesJournalPlugin.updateAllBibleNoteFrontmatter reads canonical from metadataCache and updates frontmatter via FileManager.processFrontMatter().
  • The manual YAML string builders in FrontmatterUtil.ts (buildFrontmatterString / mergeCustomFrontmatterIntoExisting) were replaced by a single applyCustomFrontmatter(fm, customYaml) that mutates the frontmatter object handed to processFrontMatter (dropping the stringifyYaml round-tripping).
  • BibleFiles.fileExistsForPassage uses getAbstractFileByPath (now synchronous), and BibleFiles.clearData deletes through FileManager.trashFile().
  • All passage/content paths run through normalizePath().

3. Rendering ESV HTML via innerHTML

File: src/components/BibleReferenceRenderer.ts (4 sites)

What's wrong: passage HTML returned by the ESV API is injected with element.innerHTML = passage.html. Flagged by no-unsanitized/property and @microsoft/sdl/no-inner-html. The content is trusted (comes from the ESV API over HTTPS), so this is currently accepted and disabled inline at each site.

Suggested fix: parse the HTML and rebuild it with Obsidian DOM helpers (createEl/createDiv), or sanitize before insertion, so no raw HTML string is written to the DOM.


4. setTimeout for scroll-to-verse

File: src/services/BibleChapterFiles.ts

The scroll-to-verse uses a fixed window.setTimeout(..., 300) to wait for the note to render before querying for the verse element. This was left as-is (only prefixed with window. for pop-out compatibility). A more robust approach would key off a render/layout event rather than a magic delay.


5. Smaller cleanups noticed in passing

  • src/services/BibleContentService.ts and src/services/ESVApiService.ts both contain // TODO notes about duplicated ESV-response → BiblePassage conversion logic that could be unified.
  • src/services/BibleChapterFiles.ts has a // TODO - logic in this class needs to move to BibleFiles note; BibleFiles.ts has TODOs for openChapterNote / openPassageNote helpers.
  • openChapterNote takes a string and re-parses it; several call sites already hold a BibleReference and could pass it directly (existing TODO).