Compare settings drafts with JSON.stringify - #31
Merged
Conversation
settingsEqual listed all six fields by hand to decide whether the Save button is enabled. Both sides are built by initialSettingsDraft from the same response.settings, and every draft update is a spread of that object, so their key order always matches and JSON.stringify compares them directly. Stringify also handles the null cases the guard covered: two nulls stringify alike, a null and an object do not. This removes a way to get the check wrong. A field added to CoslashSettings is now compared automatically, where the hand-written list would have ignored it until someone remembered to add a line. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
luhe19001
approved these changes
Aug 5, 2026
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.
Context
settingsEquallisted all six fields ofCoslashSettingsby hand to decidewhether the Save button is enabled.
Both sides of that comparison come from
initialSettingsDraft(response), andevery draft update in the dialog is a spread of that same object
(
{ ...draft, synthesis: { ...draft.synthesis, enabled } }). Spreads preserveinsertion order, and the server marshals the struct in a fixed field order, so
the two sides always share a key order.
JSON.stringifycompares themdirectly.
Changes
settingsEqual; compare withJSON.stringifyat its one call site.Stringify also covers the null cases the function's guard handled: two nulls
stringify alike, and a null and an object do not.
This removes a way to get the check wrong. A field added to
CoslashSettingsis now compared automatically, where the hand-written list would ignore it
until someone remembered to add a line.
Worth stating the failure direction, since key-order comparison is
order-sensitive: if the orders ever diverged, stringify would report unequal
for identical settings. That shows "Unsaved changes" and permits a redundant
save. It cannot report equal for settings that differ, so the Save button
cannot be wrongly disabled.
Test
npx oxlint— passed (two pre-existing Fast Refresh warnings, unchanged)npx prettier --check .— passednpx vitest run— passed (2 files, 7 tests)npm run build— passedNo unit test: the comparison is now an inline expression, and extracting it to
test would restore the function this PR removes. The frontend suite has no
React testing library and adding one for a two-line change did not seem worth
the dependency. Manual check for review: open Settings, toggle a value and back,
confirm the status text returns to "No changes" and Save disables.