fix: stable option diff to prevent spurious remounts & stale config - #71
Open
ApexYash11 wants to merge 1 commit into
Open
fix: stable option diff to prevent spurious remounts & stale config#71ApexYash11 wants to merge 1 commit into
ApexYash11 wants to merge 1 commit into
Conversation
JSON.stringify-keyed option keys were fragile: semantically identical options that differ only in declaration order produced different keys (spurious remounts that discard unsaved edits, or spurious updateOptions), undefined and function values were silently dropped (stale editor config), and circular references crashed at render. Replace the raw JSON diff with a stable canonical serializer (stableKey) that sorts object keys, treats undefined/functions as first-class values, and emits a marker for cycles instead of throwing. Add unit and component tests covering the key-order, dropped-field, and circular-reference cases; full suite stays at 100% coverage. Fixes unlayer#66.
|
@ApexYash11 is attempting to deploy a commit to the Unlayer Team on Vercel. A member of the Team first needs to authorize it. |
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.
Summary
Fixes #66 — the wrapper diffed
optionsprop changes by stringifying theoption objects with
JSON.stringifyand comparing the strings(
remountKey/updatableKeyinsrc/ImageEditor.tsx). That approach hadthree failure modes:
in property declaration order produced different keys → a spurious
remount (destroying unsaved edits — the README explicitly warns
remount-tier options discard edits, so this is data loss) or a spurious
updateOptions.undefined/ functions —JSON.stringifysilently omits them,so real changes inside
options(e.g. auserfield) were invisible to thediff → stale editor configuration without warning.
optionscrashed during render.
Change
Adds
src/optionsDiff.ts, a stable canonical serializer (stableKey) that:either key order (fixes chore(deps): Bump actions/checkout from 4 to 7 #1);
undefinedand functions as first-class values — functions are keyedby stable reference id, so a real change is never invisible (fixes chore(deps): Bump actions/setup-node from 4 to 6 #2);
circular
optionsobject can no longer crash render (fixes chore(deps): Bump codecov/codecov-action from 5 to 7 #3).ImageEditor.tsxnow usesstableKeyforremountKey,updatableKey, and themount-time
appliedUpdatableRefrecording, replacing all threeJSON.stringifysites (consistently, so initial apply / updateOptions guards stay in sync).
Tests
test/optionsDiff.test.ts— new unit tests for key-order insensitivity,dropped-field visibility, function-by-reference stability, circular-reference
safety, and every primitive branch.
test/index.test.tsx— new component tests: no remount/updateOptions onkey-order-only change, a remount is correctly detected when a
previously-
undefinedvalue becomes defined, and rendering with a circularoptionsobject mounts exactly once without crashing.Verification
npm run typecheck— passesnpm run test:coverage— 55/55 tests pass, 100% statements / branches /functions / lines across
src/**