Favorites 1.4.2: stop preset re-sync throwing a JS exception per preset - #763
Merged
Conversation
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Re-syncing Favorites presets threw a
TypeErrortwice per preset — 40 JS exceptions in the log for a 20-preset install.What was happening
rememberPresetsAfterInstall()calledsavePluginCommand()once per preset. Each call assignsDataStore.settings, and every assignment makes NotePlan writesettings.jsonand re-enter the plugin viaonSettingsUpdatedmid-statement. The still-pending assignment expression then gets evaluated in a context whereDataStoreis unbound:Note
DataStore.settingsis read successfully one line earlier, which is why "undefined" only makes sense as a re-entrancy artifact.The presets themselves always came back correctly — verified on a live install,
plugin.jsonheld all 20 custom names after the run. So this was log noise and 20x redundant work, not data loss. But it buried real errors and left the restore depending on re-entrancy ordering.The fix
Presets are read from settings, so there is nothing to write back to settings.
rememberPresetsAfterInstall()now readsplugin.jsononce, applies every preset, writes once, and never touchesDataStore.settings.savePluginCommand()is unchanged, sonp.ThemeChooserand the single-preset paths behave exactly as before.Verified against the live install
Rebuilt the plugin (which writes a fresh
plugin.jsonwith generic hidden names) and fired the re-sync, so the restore path is genuinely exercised:onSettingsUpdatedre-entriesplugin.settings[26] has no valid keyWARNTests
3 regression guards added, each confirmed to fail against the previous implementation:
plugin.jsononly once regardless of preset countDataStore.settings(counts assignments via a property setter — comparing before/after would pass spuriously, since the old code wrote back a deep-equal object)jsFunction2413 passing across
helpers+ Favorites + ThemeChooser. ESLint clean. Flow unchanged (the one pre-existingpresetChosenerror is present onmaintoo).Also here
plugin.settingshad a section heading with nokeyand notype, logginghas no valid key; skippingon every settings update — now markedseparator, like the entry below it. ThePRESETS BELOW THIS LINEdivider inplugin.commandsis marked the same way, so the app can identify dividers explicitly instead of inferring from a missingjsFunction.A companion PR does the same
separatormarking across the other 8 plugins.🤖 Generated with Claude Code