Fix premature settings evaluation at startup

This commit is contained in:
vorotamoroz
2026-08-25 13:57:04 +00:00
parent a5e1acb960
commit 537e0e42c3
6 changed files with 232 additions and 62 deletions
@@ -8,8 +8,9 @@ import { openObsidianSettings } from "@/common/obsidianSettings.ts";
export class ModuleObsidianSettingDialogue extends AbstractObsidianModule {
settingTab!: ObsidianLiveSyncSettingTab;
_everyOnloadStart(): Promise<boolean> {
_everyOnloadAfterLoadSettings(): Promise<boolean> {
this.settingTab = new ObsidianLiveSyncSettingTab(this.app, this.plugin);
this.settingTab.reloadAllSettings(true);
this.plugin.addSettingTab(this.settingTab);
eventHub.onEvent(EVENT_REQUEST_OPEN_SETTINGS, () => this.openSetting());
@@ -24,6 +25,6 @@ export class ModuleObsidianSettingDialogue extends AbstractObsidianModule {
return `${"appId" in this.app ? this.app.appId : ""}`;
}
override onBindFunction(core: LiveSyncCore, services: typeof core.services): void {
services.appLifecycle.onInitialise.addHandler(this._everyOnloadStart.bind(this));
services.appLifecycle.onSettingLoaded.addHandler(this._everyOnloadAfterLoadSettings.bind(this));
}
}
@@ -0,0 +1,91 @@
import { beforeEach, describe, expect, it, vi } from "vitest";
const settingTabState = vi.hoisted(() => ({
callOrder: [] as string[],
reloadAllSettings: vi.fn<(skipUpdate?: boolean) => void>(),
}));
const eventHubState = vi.hoisted(() => ({
onEvent: vi.fn(),
}));
vi.mock("./SettingDialogue/ObsidianLiveSyncSettingTab.ts", () => ({
ObsidianLiveSyncSettingTab: class ObsidianLiveSyncSettingTab {
reloadAllSettings(skipUpdate?: boolean) {
settingTabState.callOrder.push(`reload:${String(skipUpdate)}`);
settingTabState.reloadAllSettings(skipUpdate);
}
},
}));
vi.mock("@/common/events.ts", () => ({
EVENT_REQUEST_OPEN_SETTINGS: "request-open-settings",
eventHub: eventHubState,
}));
import { ModuleObsidianSettingDialogue } from "./ModuleObsidianSettingTab.ts";
function createModuleHarness() {
let initialisationHandler: (() => Promise<boolean>) | undefined;
let settingsLoadedHandler: (() => Promise<boolean>) | undefined;
const plugin = {
app: {},
addSettingTab: vi.fn(() => settingTabState.callOrder.push("add-setting-tab")),
};
const services = {
appLifecycle: {
onInitialise: {
addHandler: vi.fn((handler: () => Promise<boolean>) => {
initialisationHandler = handler;
}),
},
onSettingLoaded: {
addHandler: vi.fn((handler: () => Promise<boolean>) => {
settingsLoadedHandler = handler;
}),
},
},
};
const module = Object.assign(Object.create(ModuleObsidianSettingDialogue.prototype), {
plugin,
core: { services },
}) as ModuleObsidianSettingDialogue;
module.onBindFunction(module.core as never, services as never);
return {
initialisationHandler: () => initialisationHandler,
module,
plugin,
services,
settingsLoadedHandler: () => settingsLoadedHandler,
};
}
describe("ModuleObsidianSettingDialogue startup lifecycle", () => {
beforeEach(() => {
settingTabState.callOrder.length = 0;
settingTabState.reloadAllSettings.mockClear();
eventHubState.onEvent.mockClear();
});
it("registers the setting tab after persisted settings have loaded", () => {
const { initialisationHandler, services, settingsLoadedHandler } = createModuleHarness();
expect(services.appLifecycle.onInitialise.addHandler).not.toHaveBeenCalled();
expect(services.appLifecycle.onSettingLoaded.addHandler).toHaveBeenCalledOnce();
expect(initialisationHandler()).toBeUndefined();
expect(settingsLoadedHandler()).toBeTypeOf("function");
});
it("seeds the setting editor without requesting a render before registration", async () => {
const { initialisationHandler, settingsLoadedHandler } = createModuleHarness();
const handler = settingsLoadedHandler() ?? initialisationHandler();
expect(handler).toBeTypeOf("function");
await handler!();
expect(settingTabState.reloadAllSettings).toHaveBeenCalledWith(true);
expect(settingTabState.callOrder).toEqual(["reload:true", "add-setting-tab"]);
});
});
@@ -145,22 +145,36 @@ function findPage(tab: ObsidianLiveSyncSettingTab, name: string): SettingDefinit
return page;
}
function createSettingsTab(): ObsidianLiveSyncSettingTab {
const plugin = {
app: {},
core: {
settings: { ...DEFAULT_SETTINGS, useAdvancedMode: true },
confirm: {
askInPopup: vi.fn(),
type SettingsTabOptions = {
activeReplicatorGetter?: () => { syncStatus: "CONNECTED" | "PAUSED" } | undefined;
replicationStatus?: "CLOSED" | "CONNECTED" | "PAUSED";
};
function createSettingsTab(options: SettingsTabOptions = {}): ObsidianLiveSyncSettingTab {
const core = {
settings: { ...DEFAULT_SETTINGS, useAdvancedMode: true },
confirm: {
askInPopup: vi.fn(),
},
services: {
setting: {
getDeviceAndVaultName: vi.fn(() => ""),
saveSettingData: vi.fn(async () => undefined),
},
services: {
setting: {
getDeviceAndVaultName: vi.fn(() => ""),
saveSettingData: vi.fn(async () => undefined),
replicator: {
replicationStatics: {
value: { syncStatus: options.replicationStatus ?? "CLOSED" },
},
},
},
};
Object.defineProperty(core, "replicator", {
get: options.activeReplicatorGetter ?? (() => undefined),
});
const plugin = {
app: {},
core,
};
const tab = new ObsidianLiveSyncSettingTab({} as never, plugin as never);
Object.assign(tab, {
_editingSettings: { ...DEFAULT_SETTINGS, useAdvancedMode: true },
@@ -181,8 +195,27 @@ beforeEach(() => {
});
describe("ObsidianLiveSyncSettingTab native page lifecycle", () => {
it("keeps Quick Setup first while synchronisation is inactive and separates synchronisation pages from it", () => {
it("builds definitions before database readiness without requesting the active replicator", () => {
const activeReplicatorGetter = vi.fn(() => {
throw new Error("The active replicator is not ready");
});
const tab = createSettingsTab({ activeReplicatorGetter });
expect(() => tab.getSettingDefinitions()).not.toThrow();
expect(activeReplicatorGetter).not.toHaveBeenCalled();
});
it("keeps Quick Setup first while LiveSync is not configured, regardless of transient replication status", () => {
const tab = createSettingsTab({ replicationStatus: "CONNECTED" });
tab.editingSettings.isConfigured = false;
const definitions = tab.getSettingDefinitions().filter(isGroup);
expect(definitions[0]?.heading).toBe("🧙‍♂️ Quick Setup");
});
it("keeps Quick Setup first while LiveSync is not configured and separates synchronisation pages from it", () => {
const tab = createSettingsTab();
tab.editingSettings.isConfigured = false;
const definitions = tab.getSettingDefinitions().filter(isGroup);
expect(definitions.slice(0, 3).map(itemLabel)).toEqual([
@@ -192,14 +225,15 @@ describe("ObsidianLiveSyncSettingTab native page lifecycle", () => {
]);
});
it("keeps the synchronisation group first and orders General Settings before Quick Setup while synchronisation is active", () => {
it("keeps the synchronisation group first for a configured device with automatic triggers disabled", () => {
const tab = createSettingsTab();
tab.editingSettings.liveSync = true;
tab.editingSettings.isConfigured = true;
const definitions = tab.getSettingDefinitions().filter(isGroup);
expect(definitions.slice(0, 3).map(itemLabel)).toEqual([
expect(definitions.slice(0, 4).map(itemLabel)).toEqual([
"🔄 Synchronisation",
"⚙️ General Settings",
"📲 Set up other devices",
"🧙‍♂️ Quick Setup",
]);
});
@@ -40,7 +40,6 @@ import {
eventHub,
} from "@/common/events.ts";
import {
enableOnly,
// findAttrFromParent,
// getLevelStr,
setLevelClass,
@@ -587,19 +586,8 @@ export class ObsidianLiveSyncSettingTab extends PluginSettingTab {
"encrypt",
]);
}
isAnySyncEnabled() {
if (this.isConfiguredAs("isConfigured", false)) return false;
if (this.isConfiguredAs("liveSync", true)) return true;
if (this.isConfiguredAs("periodicReplication", true)) return true;
if (this.isConfiguredAs("syncOnFileOpen", true)) return true;
if (this.isConfiguredAs("syncOnSave", true)) return true;
if (this.isConfiguredAs("syncOnEditorSave", true)) return true;
if (this.isConfiguredAs("syncOnStart", true)) return true;
if (this.isConfiguredAs("syncAfterMerge", true)) return true;
if (this.isConfiguredAs("syncOnFileOpen", true)) return true;
if (this.core?.replicator?.syncStatus == "CONNECTED") return true;
if (this.core?.replicator?.syncStatus == "PAUSED") return true;
return false;
isLiveSyncConfigured() {
return this.isConfiguredAs("isConfigured", true);
}
private supportsDeclarativeSettings(): boolean {
@@ -905,13 +893,20 @@ export class ObsidianLiveSyncSettingTab extends PluginSettingTab {
getPage("help"),
getPage("change-log"),
]);
const laterGroups = [setupOtherDevices, maintenance, extraFeatures, advancedSettings, helpAndInformation];
const laterGroups = [maintenance, extraFeatures, advancedSettings, helpAndInformation];
const pendingInitialisation = this.createRebuildRequiredAction();
if (this.isAnySyncEnabled()) {
return [pendingInitialisation, synchronisation, generalSettings, quickSetup, ...laterGroups];
if (this.isLiveSyncConfigured()) {
return [
pendingInitialisation,
synchronisation,
generalSettings,
setupOtherDevices,
quickSetup,
...laterGroups,
];
}
return [pendingInitialisation, quickSetup, synchronisation, generalSettings, ...laterGroups];
return [pendingInitialisation, quickSetup, synchronisation, generalSettings, setupOtherDevices, ...laterGroups];
}
private beginRenderScope(refresh: () => void): Component {
@@ -936,8 +931,6 @@ export class ObsidianLiveSyncSettingTab extends PluginSettingTab {
this.controlledElementFunc.length = 0;
}
enableOnlySyncDisabled = enableOnly(() => !this.isAnySyncEnabled());
onlyOnP2POrCouchDB = () =>
({
visibility:
@@ -1184,7 +1177,7 @@ export class ObsidianLiveSyncSettingTab extends PluginSettingTab {
void yieldNextAnimationFrame().then(() => {
if (this.selectedScreen == "") {
if (this.isAnySyncEnabled()) {
if (this.isLiveSyncConfigured()) {
changeDisplay("20");
} else {
changeDisplay("110");
+69 -24
View File
@@ -14,7 +14,7 @@ import {
withObsidianPage,
} from "../runner/ui.ts";
import { createTemporaryVault } from "../runner/vault.ts";
import type { Locator } from "playwright";
import type { Locator, Page } from "playwright";
const uiTimeoutMs = Number(process.env.E2E_OBSIDIAN_SETTINGS_TIMEOUT_MS ?? 10000);
const settingsOnly = process.env.E2E_OBSIDIAN_SETTINGS_ONLY === "true";
@@ -34,6 +34,11 @@ type LiveSyncTestPlugin = {
applySettings: () => Promise<void>;
isP2P: boolean;
}) => Promise<unknown>;
settingTab?: {
editingSettings: { isConfigured: boolean };
initialSettings?: { isConfigured: boolean };
requestCatalogueRefresh(): void;
};
}[];
settings: {
handleFilenameCaseSensitive: boolean;
@@ -84,17 +89,14 @@ const settingsPageNames = [
"Change Log",
] as const;
async function assertDeclarativeLandingOrder(root: Locator): Promise<void> {
async function assertDeclarativeLandingOrder(root: Locator, configured: boolean): Promise<void> {
const synchronisation = ["Synchronisation", "Remote Configuration", "Sync Settings"];
const generalSettings = ["General Settings", "Appearance", "Logging", "Extra menus"];
const setup = configured
? [...synchronisation, ...generalSettings, "📲 Set up other devices", "Quick Setup"]
: ["Quick Setup", ...synchronisation, ...generalSettings, "📲 Set up other devices"];
const labels = [
"Quick Setup",
"Synchronisation",
"Remote Configuration",
"Sync Settings",
"General Settings",
"Appearance",
"Logging",
"Extra menus",
"📲 Set up other devices",
...setup,
"Maintenance and recovery",
"Maintenance",
"Hatch",
@@ -131,10 +133,31 @@ async function assertDeclarativeLandingOrder(root: Locator): Promise<void> {
}, labels);
}
async function scrollDeclarativeLandingToTop(root: Locator): Promise<void> {
const quickSetupHeading = root.locator(".setting-item-heading").filter({ hasText: "Quick Setup" }).first();
await quickSetupHeading.waitFor({ state: "visible", timeout: uiTimeoutMs });
await quickSetupHeading.scrollIntoViewIfNeeded();
async function scrollDeclarativeLandingToTop(root: Locator, configured: boolean): Promise<void> {
const firstHeading = root
.locator(".setting-item-heading")
.filter({ hasText: configured ? "Synchronisation" : "Quick Setup" })
.first();
await firstHeading.waitFor({ state: "visible", timeout: uiTimeoutMs });
await firstHeading.scrollIntoViewIfNeeded();
}
async function setConfiguredStateForLandingInspection(page: Page, configured: boolean): Promise<void> {
await page.evaluate((nextConfigured) => {
const plugin = (globalThis as ObsidianTestGlobal).app?.plugins?.plugins["obsidian-livesync"];
if (plugin === undefined) throw new Error("Self-hosted LiveSync is unavailable");
const settingDialogue = plugin.core.modules.find(
(module) => module.constructor.name === "ModuleObsidianSettingDialogue"
);
if (settingDialogue?.settingTab === undefined) {
throw new Error("The Self-hosted LiveSync setting tab is unavailable");
}
settingDialogue.settingTab.editingSettings.isConfigured = nextConfigured;
if (settingDialogue.settingTab.initialSettings !== undefined) {
settingDialogue.settingTab.initialSettings.isConfigured = nextConfigured;
}
settingDialogue.settingTab.requestCatalogueRefresh();
}, configured);
}
async function captureDeclarativeMobileLanding(): Promise<string | undefined> {
@@ -148,8 +171,8 @@ async function captureDeclarativeMobileLanding(): Promise<string | undefined> {
return undefined;
}
await settingsNavigator.returnToCatalogue();
await scrollDeclarativeLandingToTop(settingsNavigator.dialogue);
await assertDeclarativeLandingOrder(settingsNavigator.dialogue);
await scrollDeclarativeLandingToTop(settingsNavigator.dialogue, true);
await assertDeclarativeLandingOrder(settingsNavigator.dialogue, true);
const remoteConfiguration = settingsNavigator.dialogue
.locator(".setting-item-name")
.filter({ hasText: "Remote Configuration" })
@@ -437,8 +460,8 @@ async function verifyConfigDoctorFollowsCompatibilityReview(): Promise<void> {
});
}
async function verifyEffectiveSettings(): Promise<void> {
await withObsidianPage(obsidianRemoteDebuggingPort(), async (page) => {
async function verifyEffectiveSettings(): Promise<"declarative" | "imperative"> {
return await withObsidianPage(obsidianRemoteDebuggingPort(), async (page) => {
const sleepPreferences = await page.evaluate(() => {
const plugin = (globalThis as ObsidianTestGlobal).app?.plugins?.plugins["obsidian-livesync"];
if (plugin === undefined) throw new Error("Self-hosted LiveSync is unavailable");
@@ -462,6 +485,12 @@ async function verifyEffectiveSettings(): Promise<void> {
}
let settingsNavigator = await openLiveSyncSettings(page, uiTimeoutMs);
if (settingsNavigator.renderer === "imperative") {
await settingsNavigator.dialogue.screenshot({
...settingsScreenshotOptions,
path: `${diagnosticsDirectory}/settings-imperative-landing.png`,
});
}
for (const hiddenPage of ["Selector", "Customisation sync", "Advanced", "Power users", "Patches"]) {
if (await settingsNavigator.isPageListed(hiddenPage)) {
throw new Error(`${hiddenPage} was visible before its feature level was enabled.`);
@@ -568,12 +597,22 @@ async function verifyEffectiveSettings(): Promise<void> {
if (settingsNavigator.renderer === "declarative") {
await settingsNavigator.returnToCatalogue();
await scrollDeclarativeLandingToTop(settingsNavigator.dialogue);
await scrollDeclarativeLandingToTop(settingsNavigator.dialogue, true);
await settingsNavigator.dialogue.screenshot({
...settingsScreenshotOptions,
path: `${diagnosticsDirectory}/settings-declarative-landing.png`,
});
await assertDeclarativeLandingOrder(settingsNavigator.dialogue);
await assertDeclarativeLandingOrder(settingsNavigator.dialogue, true);
await setConfiguredStateForLandingInspection(page, false);
await scrollDeclarativeLandingToTop(settingsNavigator.dialogue, false);
await assertDeclarativeLandingOrder(settingsNavigator.dialogue, false);
await settingsNavigator.dialogue.screenshot({
...settingsScreenshotOptions,
path: `${diagnosticsDirectory}/settings-declarative-landing-unconfigured.png`,
});
await setConfiguredStateForLandingInspection(page, true);
await scrollDeclarativeLandingToTop(settingsNavigator.dialogue, true);
await assertDeclarativeLandingOrder(settingsNavigator.dialogue, true);
const rerunOnboarding = settingsNavigator.dialogue
.locator(".setting-item-name")
.filter({ hasText: "Rerun Onboarding Wizard" })
@@ -647,7 +686,9 @@ async function verifyEffectiveSettings(): Promise<void> {
}
}
const renderer = settingsNavigator.renderer;
await settingsNavigator.close();
return renderer;
});
}
@@ -690,7 +731,11 @@ async function verifyPendingSettingsInitialisationFlow(): Promise<{ choice: stri
has: settingsNavigator.page.getByText("Changes need to be applied!", { exact: true }),
});
await applySetting.waitFor({ state: "visible", timeout: uiTimeoutMs });
await applySetting.getByRole("button", { name: "Apply", exact: true }).click({ timeout: uiTimeoutMs });
if (settingsNavigator.renderer === "declarative") {
await applySetting.click({ timeout: uiTimeoutMs });
} else {
await applySetting.getByRole("button", { name: "Apply", exact: true }).click({ timeout: uiTimeoutMs });
}
const choiceDialogue = await waitForVisibleObsidianDialogue(
settingsNavigator.page,
@@ -798,10 +843,10 @@ async function main(): Promise<void> {
await verifyCompatibilityReview();
await verifyConfigDoctorFollowsCompatibilityReview();
}
await verifyEffectiveSettings();
const settingsRenderer = await verifyEffectiveSettings();
const initialisation = await verifyPendingSettingsInitialisationFlow();
const p2pInitialisation = await captureP2PSettingsInitialisationDialogue();
const mobileLanding = await captureDeclarativeMobileLanding();
const mobileLanding = settingsRenderer === "declarative" ? await captureDeclarativeMobileLanding() : undefined;
if (mobileLanding) console.log(`Declarative mobile settings landing page: ${mobileLanding}`);
console.log(
`Pending-settings initialisation screenshots: ${initialisation.choice}, ${initialisation.fallback}, ${p2pInitialisation}`
+6
View File
@@ -12,6 +12,12 @@ Earlier releases remain available in the 1.0 release history, the 1.0 preview hi
## Unreleased
### Interface and translation
#### Fixed
- Obsidian 1.13 settings discovery now waits until persisted settings have loaded and no longer queries the active replicator before database initialisation, preventing a spurious start-up warning.
## 1.0.19
25th August, 2026