Skip to content

Notebook Navigator integration misses mid-session enable, and polls on startup #425

Description

@churnish

Notebook Navigator's folder menu items are registered from a polling loop started in onLayoutReady (main.ts:322-339). This has two problems, one of which is a hard failure.

1. Enabling NN mid-session is never detected

if (!this.registerNotebookNavigatorMenus()) {
  // NN not installed — skip retry loop
  if (!this.app.plugins?.plugins?.['notebook-navigator']) return;

If NN is absent from the plugin registry at layout-ready, we bail with no retry and no listener. Enable NN afterwards and Dynamic Views' folder menu items never appear until Obsidian is restarted. There is no feedback that anything is wrong.

2. The retry loop is a timer, and it can still lose

When NN is in the registry but its API isn't ready, we poll every 500 ms for 10 s — up to 20 timer wakeups every startup, and a hard give-up if NN's async onload runs past the deadline.

Proposed fix: replace the loop with Obsidian's plugin-changed event

app.plugins is an Events emitter and fires changed whenever any plugin is enabled or disabled. Verified empirically against a running Obsidian:

Check Result
app.plugins.on / off / offref exist yes
enablePlugin() / disablePlugin() fire it yes, via a debounced didChange()
Lag from enable to event 12–18 ms
Is api.menus ready when it fires? yes, already truthy
Handler cost when nothing changed 0.017 µs per fire

So the whole loop collapses to:

this.app.workspace.onLayoutReady(() => {
  this.registerNotebookNavigatorMenus();
  this.registerEvent(
    this.app.plugins.on('changed', () => {
      this.registerNotebookNavigatorMenus();
    })
  );
});

registerNotebookNavigatorMenus() already guards on API identity (nnRegisteredApi, main.ts:380-381), so repeat calls are no-ops and it stays idempotent. changed fires for every plugin toggle in the vault, but the identity check short-circuits before doing any work.

This also fixes both problems at once: no timers, no deadline, and mid-session enable works.

One thing to handle that the current code doesn't

Disabling and re-enabling NN produces a new API object — NN rebuilds its menu registry. The existing dispose returned by registerFolderMenu is passed straight to this.register(dispose) (main.ts:435), which only runs at plugin unload, so repeated NN toggling accumulates stale disposers. Worth holding them in an array instead and disposing before re-registering, with the calls wrapped in try/catch since a disposer belonging to an unloaded NN instance may throw.

Typing note

app.plugins is undocumented and absent from obsidian.d.ts, so the changed event needs a local declaration:

interface App {
  plugins: {
    plugins: Record<string, unknown>;
    on(name: 'changed', callback: () => unknown): EventRef;
  };
}

Failure mode if Obsidian ever drops the event is benign: Events.on accepts unknown event names without throwing (verified), so the listener would simply never fire and behaviour degrades to today's startup-only registration.

Unrelated cleanup spotted nearby

src/core/notebook-navigator.ts and src/utils/notebook-navigator.ts are byte-identical duplicates, imported from different call sites (src/bases/ uses one, src/shared/card-renderer.tsx the other). Worth collapsing to one module while touching this area.

Context

This mirrors a fix just made in first-line-is-title, which had the same startup-only registration bug against the same NN API.

🤖 Generated with Claude Code

Metadata

Metadata

Assignees

No one assigned

    Labels

    Bug reportSomething isn't working as expected🟢 Low priorityWould be nice but not important

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions