Call api.storeSetting only when setting value changes (#2119)

src/stores/settingStore.ts

@@ -24,13 +24,13 @@ function tryMigrateDeprecatedValue(setting: SettingParams, value: any) {
   return setting?.migrateDeprecatedValue?.(value) ?? value
 }
 
-function onChange(setting: SettingParams, value: any, oldValue: any) {
-  if (setting?.onChange && value !== oldValue) {
-    setting.onChange(value)
-    // Backward compatibility with old settings dialog.
-    // Some extensions still listens event emitted by the old settings dialog.
-    app.ui.settings.dispatchChange(setting.id, value, oldValue)
+function onChange(setting: SettingParams, newValue: any, oldValue: any) {
+  if (setting?.onChange) {
+    setting.onChange(newValue, oldValue)
   }
+  // Backward compatibility with old settings dialog.
+  // Some extensions still listens event emitted by the old settings dialog.
+  app.ui.settings.dispatchChange(setting.id, newValue, oldValue)
 }
 
 export const useSettingStore = defineStore('setting', () => {
@@ -75,9 +75,10 @@ export const useSettingStore = defineStore('setting', () => {
    */
   async function set<K extends keyof Settings>(key: K, value: Settings[K]) {
     const newValue = tryMigrateDeprecatedValue(settingsById.value[key], value)
-    const oldValue = settingValues.value[key]
-    onChange(settingsById.value[key], newValue, oldValue)
+    const oldValue = get(key)
+    if (newValue === oldValue) return
 
+    onChange(settingsById.value[key], newValue, oldValue)
     settingValues.value[key] = newValue
     await api.storeSetting(key, newValue)
   }

tests-ui/tests/store/settingStore.test.ts

@@ -1,6 +1,7 @@
 import { createPinia, setActivePinia } from 'pinia'
 
 import { api } from '@/scripts/api'
+import { app } from '@/scripts/app'
 import { getSettingInfo, useSettingStore } from '@/stores/settingStore'
 import type { SettingParams } from '@/types/settingTypes'
 
@@ -123,6 +124,7 @@ describe('useSettingStore', () => {
 
     it('should set value and trigger onChange', async () => {
       const onChangeMock = jest.fn()
+      const dispatchChangeMock = app.ui.settings.dispatchChange as jest.Mock
       const setting: SettingParams = {
         id: 'test.setting',
         name: 'test.setting',
@@ -131,12 +133,32 @@ describe('useSettingStore', () => {
         onChange: onChangeMock
       }
       store.addSetting(setting)
+      // Adding the new setting should trigger onChange
+      expect(onChangeMock).toHaveBeenCalledTimes(1)
+      expect(dispatchChangeMock).toHaveBeenCalledTimes(1)
 
       await store.set('test.setting', 'newvalue')
 
       expect(store.get('test.setting')).toBe('newvalue')
-      expect(onChangeMock).toHaveBeenCalledWith('newvalue')
+      expect(onChangeMock).toHaveBeenCalledWith('newvalue', 'default')
+      expect(onChangeMock).toHaveBeenCalledTimes(2)
+      expect(dispatchChangeMock).toHaveBeenCalledTimes(2)
       expect(api.storeSetting).toHaveBeenCalledWith('test.setting', 'newvalue')
+
+      // Set the same value again, it should not trigger onChange
+      await store.set('test.setting', 'newvalue')
+      expect(onChangeMock).toHaveBeenCalledTimes(2)
+      expect(dispatchChangeMock).toHaveBeenCalledTimes(2)
+
+      // Set a different value, it should trigger onChange
+      await store.set('test.setting', 'differentvalue')
+      expect(onChangeMock).toHaveBeenCalledWith('differentvalue', 'newvalue')
+      expect(onChangeMock).toHaveBeenCalledTimes(3)
+      expect(dispatchChangeMock).toHaveBeenCalledTimes(3)
+      expect(api.storeSetting).toHaveBeenCalledWith(
+        'test.setting',
+        'differentvalue'
+      )
     })
   })
 })