From 041af6859f110983b95d245e0fd5d88e84a8979e Mon Sep 17 00:00:00 2001 From: Huang Xin Date: Thu, 14 May 2026 00:25:57 +0800 Subject: [PATCH] fix(sync): publish custom css settings after applying css, closes #4146 (#4151) --- .../src/__tests__/helpers/settings.test.ts | 120 ++++++++++++++++++ apps/readest-app/src/helpers/settings.ts | 15 ++- 2 files changed, 132 insertions(+), 3 deletions(-) create mode 100644 apps/readest-app/src/__tests__/helpers/settings.test.ts diff --git a/apps/readest-app/src/__tests__/helpers/settings.test.ts b/apps/readest-app/src/__tests__/helpers/settings.test.ts new file mode 100644 index 00000000..1530e7c6 --- /dev/null +++ b/apps/readest-app/src/__tests__/helpers/settings.test.ts @@ -0,0 +1,120 @@ +import { describe, test, expect, beforeEach, vi } from 'vitest'; + +const getViewSettingsMock = vi.fn<(bookKey: string) => { isGlobal?: boolean } | undefined>( + () => undefined, +); +const setViewSettingsMock = vi.fn(); +const getViewMock = vi.fn(() => null); +const getViewStateMock = vi.fn(() => undefined); + +vi.mock('@/store/readerStore', () => ({ + useReaderStore: { + getState: () => ({ + bookKeys: [], + getView: getViewMock, + getViewState: getViewStateMock, + getViewSettings: getViewSettingsMock, + setViewSettings: setViewSettingsMock, + }), + }, +})); + +vi.mock('@/store/bookDataStore', () => ({ + useBookDataStore: { + getState: () => ({ + getConfig: vi.fn(() => null), + saveConfig: vi.fn(), + }), + }, +})); + +vi.mock('@/utils/style', () => ({ + getStyles: vi.fn(() => ''), +})); + +import { saveViewSettings } from '@/helpers/settings'; +import { useSettingsStore } from '@/store/settingsStore'; +import type { EnvConfigType } from '@/services/environment'; +import type { SystemSettings } from '@/types/settings'; + +const envConfig = {} as EnvConfigType; + +const makeSettings = (): SystemSettings => + ({ + globalViewSettings: { userStylesheet: '', userUIStylesheet: '' }, + }) as unknown as SystemSettings; + +beforeEach(() => { + getViewSettingsMock.mockReset(); + getViewSettingsMock.mockReturnValue(undefined); + setViewSettingsMock.mockReset(); + useSettingsStore.setState({ + settings: makeSettings(), + setSettings: (s: SystemSettings) => useSettingsStore.setState({ settings: s }), + saveSettings: vi.fn(async () => {}), + } as unknown as ReturnType); +}); + +describe('saveViewSettings', () => { + test('global write swaps the settings reference so replicaSettingsSync subscribers fire', async () => { + // Mirrors the gating subscriber installed by replicaSettingsSync.initSettingsSync. + // The publish path is bypassed entirely when this never fires, which is exactly + // why the MiscPanel "Apply" button was failing to ship custom CSS to the server + // until some unrelated setSettings call (e.g. dictionary provider toggle) + // happened to create a new top-level reference and trigger a diff sweep. + const referenceChanges: SystemSettings[] = []; + const unsubscribe = useSettingsStore.subscribe((state, prev) => { + if (state.settings && state.settings !== prev?.settings) { + referenceChanges.push(state.settings); + } + }); + + try { + await saveViewSettings(envConfig, 'book-1', 'userStylesheet', 'body { color: red; }'); + } finally { + unsubscribe(); + } + + expect(referenceChanges).toHaveLength(1); + expect(referenceChanges[0]!.globalViewSettings.userStylesheet).toBe('body { color: red; }'); + }); + + test('global write persists with the same new reference passed to setSettings', async () => { + let savedSettings: SystemSettings | null = null; + const saveSettingsMock = vi.fn(async (_env: EnvConfigType, s: SystemSettings) => { + savedSettings = s; + }); + useSettingsStore.setState({ + saveSettings: saveSettingsMock, + } as unknown as ReturnType); + + await saveViewSettings(envConfig, 'book-1', 'userUIStylesheet', '.app { background: black; }'); + + expect(saveSettingsMock).toHaveBeenCalledTimes(1); + expect(savedSettings!.globalViewSettings.userUIStylesheet).toBe('.app { background: black; }'); + expect(savedSettings).toBe(useSettingsStore.getState().settings); + }); + + test('per-book write (isGlobal=false) does not touch the global settings reference', async () => { + getViewSettingsMock.mockImplementation(() => ({ isGlobal: false })); + const initial = useSettingsStore.getState().settings; + const referenceChanges: SystemSettings[] = []; + const unsubscribe = useSettingsStore.subscribe((state, prev) => { + if (state.settings && state.settings !== prev?.settings) { + referenceChanges.push(state.settings); + } + }); + + try { + await saveViewSettings(envConfig, 'book-1', 'userStylesheet', 'body { color: blue; }'); + } finally { + unsubscribe(); + } + + // Per-book writes go through applyViewSettings on the readerStore — they + // must NOT publish global settings, otherwise per-book overrides would leak + // into the cross-device globals. + expect(referenceChanges).toHaveLength(0); + expect(useSettingsStore.getState().settings).toBe(initial); + }); +}); diff --git a/apps/readest-app/src/helpers/settings.ts b/apps/readest-app/src/helpers/settings.ts index f774d21c..f8187c13 100644 --- a/apps/readest-app/src/helpers/settings.ts +++ b/apps/readest-app/src/helpers/settings.ts @@ -38,13 +38,22 @@ export const saveViewSettings = async ( const isSettingsGlobal = getViewSettings(bookKey)?.isGlobal ?? true; if (isSettingsGlobal && !skipGlobal) { - settings.globalViewSettings[key] = value; - setSettings(settings); + // Build a NEW settings object (and a NEW globalViewSettings) so the + // settingsStore subscriber that gates replica push fires — it compares + // `state.settings !== prev.settings`, so an in-place mutation followed + // by setSettings(same_ref) silently bypasses the publish path and + // whitelisted writes (userStylesheet, userUIStylesheet) only ship + // on the next unrelated setSettings call. + const nextSettings: SystemSettings = { + ...settings, + globalViewSettings: { ...settings.globalViewSettings, [key]: value }, + }; + setSettings(nextSettings); for (const bookKey of bookKeys) { await applyViewSettings(bookKey); } - await saveSettings(envConfig, settings); + await saveSettings(envConfig, nextSettings); } else if (bookKey) { await applyViewSettings(bookKey); }