From a6b97ddb3b1c8bfdc7cd37b1b9fe9bb92ed23d93 Mon Sep 17 00:00:00 2001 From: vorotamoroz Date: Fri, 6 Nov 2026 12:17:31 +0000 Subject: [PATCH] refactor: register log cleanup through the host API Delegate display, subscription, and collector cleanup through APIService instead of registering directly on the Obsidian plug-in. The Obsidian adapter forwards the same callbacks to native component ownership. Preserve the separate log resource which remains alive throughout asynchronous shutdown, and cover native delegation and renderer cleanup registration. --- src/modules/features/ModuleLog.ts | 10 +++++----- src/modules/features/ModuleLog.unit.spec.ts | 20 +++++++++++++++++++ src/modules/services/ObsidianAPIService.ts | 5 +++++ .../services/ObsidianAPIService.unit.spec.ts | 13 ++++++++++++ 4 files changed, 43 insertions(+), 5 deletions(-) diff --git a/src/modules/features/ModuleLog.ts b/src/modules/features/ModuleLog.ts index c42974d4..45aa95d2 100644 --- a/src/modules/features/ModuleLog.ts +++ b/src/modules/features/ModuleLog.ts @@ -168,7 +168,7 @@ export class ModuleLog extends AbstractObsidianModule { observeForLogs() { const registerDisplay = (display: T): T => { - this.plugin.register(() => display.dispose()); + this.services.API.registerCleanup(() => display.dispose()); return display; }; const labelReplication = registerDisplay( @@ -211,12 +211,12 @@ export class ModuleLog extends AbstractObsidianModule { ); const p2pProgress = registerDisplay(createP2PProgressDisplay(this.p2pLogCollector.p2pReplicationStats)); const p2pIsPrimary = reactiveSource(this.settings.remoteType === REMOTE_P2P); - this.plugin.register( + this.services.API.registerCleanup( this.services.context.events.onEvent(EVENT_SETTING_SAVED, (settings) => { p2pIsPrimary.value = settings.remoteType === REMOTE_P2P; }) ); - this.plugin.register(() => this.p2pLogCollector.dispose()); + this.services.API.registerCleanup(() => this.p2pLogCollector.dispose()); const replicationStatLabel = computed(() => projectReplicationStatusLabel( this.services.replicator.replicationStatics.value, @@ -530,12 +530,12 @@ ${stringifyYaml(info)} if (this.statusLine) { const renderer = createStatusBarRenderer(this.statusLine); this.statusLineRenderer = renderer; - this.plugin.register(() => renderer.dispose()); + this.services.API.registerCleanup(() => renderer.dispose()); } if (this.statusBar) { const renderer = createStatusBarRenderer(this.statusBar); this.statusBarRenderer = renderer; - this.plugin.register(() => renderer.dispose()); + this.services.API.registerCleanup(() => renderer.dispose()); } this.adjustStatusDivPosition(); this._log("Log module loaded", LOG_LEVEL_INFO); diff --git a/src/modules/features/ModuleLog.unit.spec.ts b/src/modules/features/ModuleLog.unit.spec.ts index bf2f7d44..9071814a 100644 --- a/src/modules/features/ModuleLog.unit.spec.ts +++ b/src/modules/features/ModuleLog.unit.spec.ts @@ -8,6 +8,9 @@ vi.mock("@/deps.ts", () => ({ stringifyYaml: vi.fn(), })); vi.mock("./Log/LogPaneView.ts", () => ({ LogPaneView: class {}, VIEW_TYPE_LOG: "log" })); +vi.mock("./StatusBarRenderer.ts", () => ({ + createStatusBarRenderer: vi.fn(() => ({ dispose: vi.fn() })), +})); vi.mock("@/common/reportTool.ts", () => ({ generateReport: vi.fn() })); vi.mock("@/common/utils.ts", () => ({ isValidPath: () => true })); vi.mock("@/common/translation", () => ({ $msg: (key: string) => key })); @@ -36,6 +39,7 @@ async function createHarness() { registerWindow: vi.fn(), addRibbonIcon: vi.fn(), registerProtocolHandler: vi.fn(), + registerCleanup: vi.fn<(callback: () => void) => void>(), }, context: { events: createLiveSyncEventHub(), notices: { hide: vi.fn() } }, appLifecycle: { @@ -64,6 +68,22 @@ describe("ModuleLog service connection", () => { vi.unstubAllGlobals(); }); + it("registers status renderer cleanup through the API without native plug-in registration", async () => { + const { services, module } = await createHarness(); + const { createStatusBarRenderer } = await import("./StatusBarRenderer.ts"); + const factory = vi.mocked(createStatusBarRenderer); + factory.mockClear(); + Object.assign(module, { statusLine: {}, statusBar: {} }); + await services.appLifecycle.onSettingLoaded(); + expect(factory).toHaveBeenCalledTimes(2); + expect(services.API.registerCleanup).toHaveBeenCalledTimes(2); + const renderers = factory.mock.results.map((result) => result.value); + for (const renderer of renderers) expect(renderer.dispose).not.toHaveBeenCalled(); + for (const [cleanup] of services.API.registerCleanup.mock.calls) cleanup(); + for (const renderer of renderers) expect(renderer.dispose).toHaveBeenCalledOnce(); + await module.logResource.dispose(); + }); + it("routes accepted P2P diagnostics only through their owning context and disposes the subscription", async () => { const first = await createHarness(); const second = await createHarness(); diff --git a/src/modules/services/ObsidianAPIService.ts b/src/modules/services/ObsidianAPIService.ts index 5f5a5862..d188d589 100644 --- a/src/modules/services/ObsidianAPIService.ts +++ b/src/modules/services/ObsidianAPIService.ts @@ -135,6 +135,11 @@ export class ObsidianAPIService extends InjectableAPIService void): void { + // Keep disposal owned by the native component, preserving its unload timing. + this.context.plugin.register(callback); + } + /** * In Obsidian, we will use the native `requestUrl` function as the default fetch handler, * to address unavoidable CORS issues. diff --git a/src/modules/services/ObsidianAPIService.unit.spec.ts b/src/modules/services/ObsidianAPIService.unit.spec.ts index 9eb4aa53..60d1f1b9 100644 --- a/src/modules/services/ObsidianAPIService.unit.spec.ts +++ b/src/modules/services/ObsidianAPIService.unit.spec.ts @@ -248,3 +248,16 @@ describe("ObsidianAPIService.nativeFetch cache control", () => { expect(sentHeaders().has("pragma")).toBe(false); }); }); + +describe("ObsidianAPIService.registerCleanup", () => { + it("delegates the original callback to native component ownership without invoking it immediately", () => { + const register = vi.fn(); + const service = new ObsidianAPIService({ app: {}, plugin: { register } } as unknown as ObsidianServiceContext); + const cleanup = vi.fn(); + expect(service.registerCleanup(cleanup)).toBeUndefined(); + expect(register).toHaveBeenCalledExactlyOnceWith(cleanup); + expect(cleanup).not.toHaveBeenCalled(); + register.mock.calls[0][0](); + expect(cleanup).toHaveBeenCalledOnce(); + }); +});