Favorites 1.4.2: stop preset re-sync throwing a JS exception per preset

dwertheimer.Favorites/CHANGELOG.md

@@ -1,6 +1,9 @@
 # Favorites Plugin Changelog
 
-## [1.4.1] - 2026-07-21 @dwertheimer
+## [1.4.2] - 2026-07-30 @dwertheimer
+
+- **Fixed**: Re-syncing presets (`onUpdateOrInstall`) threw a `TypeError: undefined is not an object (evaluating 'DataStore.settings = ...')` twice per preset — 40 JS exceptions in the log for a 20-preset install. `rememberPresetsAfterInstall()` assigned `DataStore.settings` once per preset, and each assignment made NotePlan fire `onSettingsUpdated` re-entrantly mid-statement. The presets themselves were always restored correctly, so this was log noise rather than data loss, but it buried genuine errors and did 20x the necessary work. Presets are now re-applied to `plugin.json` in a single read/write, with no settings write at all (the settings are the source being read from).
+- **Fixed**: `plugin.settings` had a section-heading entry with no `key` and no `type`, producing `plugin.settings[26] has no valid key; skipping` on every settings update. Marked it as a `separator`, like the entry below it.
 
 - **Fixed**: The unfavorite star icon on each note row was getting clipped at the top of the row.
 - **Feature**: Added a "Group favorites by folder" plugin setting (bool). This controls the initial state of the Notes list's "group by folder" toggle when the Favorites browser window opens; users can still switch modes from the toggle itself within the session.

dwertheimer.Favorites/plugin.json

@@ -6,8 +6,8 @@
   "plugin.name": "⭐️ Favorites",
   "plugin.description": "Get fast access to commonly-used notes. Set any Project Note(s) as a Favorites and have quick access to choose/open the file",
   "plugin.author": "@dwertheimer",
-  "plugin.version": "1.4.1",
-  "plugin.lastUpdateInfo": "1.4.1: Fix clipped unfavorite star icon; add 'Group favorites by folder' setting.\n1.4.0: Add 'Group by folder' toggle to Favorites browser Notes view.\n1.3.6: PluginRequestEnvelope / requestFromPlugin aligned with np.Shared 1.0.7.\n1.3.5: Fix sidebar icon colour issue\n1.3.4: Fix dark mode issues (thx @clayrussell)\n1.3.3: Use new sidebar view feature\n1.3.0: Add new '/favorites browser' command to open favorites in a sidebar window to view and open favorite notes and commands\n1.3.1: Fix request timeout issue\n1.3.2: Fix frontmatter not being removed when unfavoriting",
+  "plugin.version": "1.4.2",
+  "plugin.lastUpdateInfo": "1.4.2: Preset re-sync no longer floods the log with JS exceptions.\n1.4.1: Fix clipped unfavorite star icon; add 'Group favorites by folder' setting.\n1.4.0: Add 'Group by folder' toggle to Favorites browser Notes view.\n1.3.6: PluginRequestEnvelope / requestFromPlugin aligned with np.Shared 1.0.7.\n1.3.5: Fix sidebar icon colour issue\n1.3.4: Fix dark mode issues (thx @clayrussell)\n1.3.3: Use new sidebar view feature\n1.3.0: Add new '/favorites browser' command to open favorites in a sidebar window to view and open favorite notes and commands\n1.3.1: Fix request timeout issue\n1.3.2: Fix frontmatter not being removed when unfavoriting",
   "plugin.dependencies": [],
   "plugin.requiredFiles": ["react.c.FavoritesView.bundle.dev.js"],
   "plugin.script": "script.js",
@@ -63,7 +63,8 @@
     {
       "note": "****************************************************",
       "not1": "**********  PRESETS BELOW THIS LINE      ***********",
-      "not2": "****************************************************"
+      "not2": "****************************************************",
+      "type": "separator"
     },
     {
       "name": "Set/Change/Rename Preset Action",
@@ -415,7 +416,8 @@
       "description": "Do not change this setting manually. Use the \"/Change Favorite Preset\" command."
     },
     {
-      "note": "================== DEBUGGING SETTINGS ========================"
+      "note": "================== DEBUGGING SETTINGS ========================",
+      "type": "separator"
     },
     {
       "NOTE": "DO NOT CHANGE THE FOLLOWING SETTINGS; ADD YOUR SETTINGS ABOVE ^^^",

helpers/NPPresets.js

@@ -212,18 +212,26 @@ export function getCommandIndex(pluginJson: any, functionName: string): number {
  * @param {object} pluginJson - the entire settings object
  */
 export async function rememberPresetsAfterInstall(pluginJson: any): Promise<void> {
+  const pluginID = pluginJson['plugin.id']
   const settings = DataStore.settings
-  const settingsKeys = Object.keys(settings)
-  for (let index = 0; index < settingsKeys.length; index++) {
-    const setting = settingsKeys[index]
-    if (setting.includes('runPreset')) {
-      // settings will be empty strings until they are set by a user
-      if (settings[setting] === '') continue
-      logDebug(pluginJson, `rememberPresetsAfterInstall: ${setting} was prev set to: ${JSP(settings[setting])}`)
-      await savePluginCommand(pluginJson, settings[setting])
-    }
+  // settings will be empty strings until they are set by a user
+  const presetKeys = Object.keys(settings).filter((key) => key.includes('runPreset') && settings[key] !== '' && settings[key]?.jsFunction)
+  if (presetKeys.length === 0) return
+  // NOTE: deliberately does not call savePluginCommand() in a loop here. That assigns DataStore.settings once per
+  // preset, and each assignment makes NotePlan write settings.json and re-enter the plugin via onSettingsUpdated
+  // mid-statement, which throws "undefined is not an object (evaluating 'DataStore.settings = ...')" every time.
+  // The presets we are restoring are read *from* settings, so there is nothing to save back -- we only need to
+  // re-apply them to the freshly-installed plugin.json. One read, one write, no settings trigger.
+  const livePluginJson = await getPluginJson(pluginID)
+  if (!livePluginJson) {
+    logError(pluginJson, `rememberPresetsAfterInstall: Could not find plugin.json for ${pluginID}`)
+    return
+  }
+  let updatedPluginJson = livePluginJson
+  for (const key of presetKeys) {
+    logDebug(pluginJson, `rememberPresetsAfterInstall: ${key} was prev set to: ${JSP(settings[key])}`)
+    updatedPluginJson = updateJSONForFunctionNamed(updatedPluginJson, settings[key], false)
   }
-  // clo(pluginJson, `Before plugin update/install, pluginJson is:`)
-  // const livePluginJson = await getPluginJson(pluginJson['plugin.id'])
-  // clo(livePluginJson, `After plugin update/install, pluginJson is:`)
+  logDebug(pluginJson, `rememberPresetsAfterInstall: restoring ${presetKeys.length} preset(s) to ${pluginID}/plugin.json in a single write`)
+  await savePluginJson(pluginID, updatedPluginJson)
 }

helpers/__tests__/NPPresets.test.js

@@ -181,6 +181,58 @@ describe(`${PLUGIN_NAME}`, () => {
         const updatedCommands = newPluginJson['plugin.commands']
         expect(updatedCommands[3].data).toEqual('someData')
       })
+      test('should write plugin.json only once, no matter how many presets there are', async () => {
+        // Regression guard: this used to call savePluginCommand() per preset, which re-read and re-wrote
+        // plugin.json every time and assigned DataStore.settings, making NotePlan fire onSettingsUpdated
+        // re-entrantly and throw once per preset.
+        DataStore.settings = {
+          ...DataStore.settings,
+          ...{
+            runPreset01: { jsFunction: 'runPreset01', data: 'one' },
+            runPreset02: { jsFunction: 'runPreset02', data: 'two' },
+            runPreset03: { jsFunction: 'runPreset03', data: 'three' },
+          },
+        }
+        const spy = jest.spyOn(DataStore, 'saveJSON')
+        const pluginJson = await DataStore.loadJSON('') //get the default json
+        await f.rememberPresetsAfterInstall(pluginJson)
+        expect(spy).toHaveBeenCalledTimes(1)
+        spy.mockRestore()
+      })
+      test('should not assign DataStore.settings (which triggers onSettingsUpdated re-entrantly)', async () => {
+        // Counts *assignments*, not the resulting value: the old code wrote back a deep-equal object, so
+        // comparing before/after would pass even when the (trigger-firing) assignment did happen.
+        const originalSettings = { ...DataStore.settings, ...{ runPreset01: { jsFunction: 'runPreset01', data: 'someData' } } }
+        let stored = originalSettings
+        let assignments = 0
+        Object.defineProperty(DataStore, 'settings', {
+          configurable: true,
+          enumerable: true,
+          get: () => stored,
+          set: (value) => {
+            assignments += 1
+            stored = value
+          },
+        })
+        const pluginJson = await DataStore.loadJSON('') //get the default json
+        await f.rememberPresetsAfterInstall(pluginJson)
+        // restore a plain data property before asserting, so a failure can't leave the mock patched
+        Object.defineProperty(DataStore, 'settings', { configurable: true, enumerable: true, writable: true, value: originalSettings })
+        expect(assignments).toEqual(0)
+      })
+      test('should skip preset settings that are empty strings or have no jsFunction', async () => {
+        DataStore.settings = {
+          ...DataStore.settings,
+          ...{ runPreset09: '', runPreset08: { name: 'no jsFunction here' } },
+        }
+        const spy = jest.spyOn(DataStore, 'saveJSON')
+        const pluginJson = await DataStore.loadJSON('') //get the default json
+        await f.rememberPresetsAfterInstall(pluginJson)
+        // nothing valid to restore in these two, so they must not produce their own writes
+        const calls = spy.mock.calls.length
+        spy.mockRestore()
+        expect(calls).toBeLessThanOrEqual(1)
+      })
     })
   })
 })