From fc1f397cfe205ef8f8c818cef02333a0cfec180a Mon Sep 17 00:00:00 2001 From: David Wertheimer Date: Thu, 30 Jul 2026 11:20:06 -0700 Subject: [PATCH 1/2] Favorites 1.4.2: stop preset re-sync throwing a JS exception per preset rememberPresetsAfterInstall() called savePluginCommand() once per preset. Each call assigned DataStore.settings, and every assignment makes NotePlan write settings.json and re-enter the plugin via onSettingsUpdated mid-statement -- so the pending assignment expression was re-evaluated in a context where DataStore is unbound and threw "undefined is not an object (evaluating 'DataStore.settings = ...')". A 20-preset re-sync logged 20 onSettingsUpdated invocations and 40 JS exceptions, and re-read plus re-wrote plugin.json 20 times. The presets themselves always came back correctly (verified against the live install: plugin.json held all 20 custom names after the run), so this was noise and wasted work rather than data loss -- but it buried real errors in the log and left the restore depending on re-entrancy ordering. Presets are read *from* settings, so there is nothing to write back to settings. Now reads plugin.json once, applies every preset, writes once, and never touches DataStore.settings. savePluginCommand() is unchanged, so np.ThemeChooser and the single-preset paths behave exactly as before. Also marks the "DEBUGGING SETTINGS" heading in plugin.settings as a separator; without a key or type it logged "plugin.settings[26] has no valid key; skipping" on every settings update. Tests: 3 regression guards added (single write, no settings assignment, skips empty and malformed presets); all three fail against the previous implementation. 2413 passing across helpers + Favorites + ThemeChooser. Co-Authored-By: Claude Opus 5 (1M context) --- dwertheimer.Favorites/CHANGELOG.md | 5 ++- dwertheimer.Favorites/plugin.json | 7 ++-- helpers/NPPresets.js | 32 +++++++++++------- helpers/__tests__/NPPresets.test.js | 52 +++++++++++++++++++++++++++++ 4 files changed, 80 insertions(+), 16 deletions(-) diff --git a/dwertheimer.Favorites/CHANGELOG.md b/dwertheimer.Favorites/CHANGELOG.md index 25b3c4401..051c730b1 100644 --- a/dwertheimer.Favorites/CHANGELOG.md +++ b/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. diff --git a/dwertheimer.Favorites/plugin.json b/dwertheimer.Favorites/plugin.json index ce654fd3c..8f4a181d6 100644 --- a/dwertheimer.Favorites/plugin.json +++ b/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", @@ -415,7 +415,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 ^^^", diff --git a/helpers/NPPresets.js b/helpers/NPPresets.js index ccf50d143..69e13b10a 100644 --- a/helpers/NPPresets.js +++ b/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 { + 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) } diff --git a/helpers/__tests__/NPPresets.test.js b/helpers/__tests__/NPPresets.test.js index 468fdbea2..bc58a4884 100644 --- a/helpers/__tests__/NPPresets.test.js +++ b/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) + }) }) }) }) From 58ab071023682de991ca21d04bc9ca564ed09f70 Mon Sep 17 00:00:00 2001 From: David Wertheimer Date: Thu, 30 Jul 2026 11:24:36 -0700 Subject: [PATCH 2/2] Favorites: mark the PRESETS divider in plugin.commands as a separator Gives the app a reliable way to tell a comment/divider entry apart from a real command, rather than having to infer it from a missing jsFunction. Co-Authored-By: Claude Opus 5 (1M context) --- dwertheimer.Favorites/plugin.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dwertheimer.Favorites/plugin.json b/dwertheimer.Favorites/plugin.json index 8f4a181d6..0d9542659 100644 --- a/dwertheimer.Favorites/plugin.json +++ b/dwertheimer.Favorites/plugin.json @@ -63,7 +63,8 @@ { "note": "****************************************************", "not1": "********** PRESETS BELOW THIS LINE ***********", - "not2": "****************************************************" + "not2": "****************************************************", + "type": "separator" }, { "name": "Set/Change/Rename Preset Action",