Repository navigation
fix: finalize drafts nested in inserted values after the array methods plugin moves them - #1371
Open
waw666waw666 wants to merge 1 commit into
Open
waw666waw666 wants to merge 1 commit into
waw666waw666 wants to merge 1 commit into
Conversation
…s plugin moves them
This branch has not been deployed
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.
With
enableArrayMethods(), a draft that sits inside a new plain value inserted into an array is not finalized if a later shifting or reordering call moves that value. The result then holds a revoked proxy:Without the plugin the same recipe works, because the native methods write each moved element through the
settrap and register a new callback for its new index.Cause
handleCrossReferenceregisters a callback for a non-draft value that may contain drafts, keyed by the index it was inserted at. At finalization the callback only processes the value if it is still at that index (and that index is inassigned_). The plugin'sshift,unshift,splice,sortandreversemove elements oncopy_directly and setallIndicesReassigned_instead, so the value is no longer at its original index, the callback skips it, and the nested draft is revoked without being replaced.Fix
When the value is not at its key and the target is an array whose indices were all reassigned, check whether the array still contains the value and treat it as assigned in that case. The fast path (value still at its key) is unchanged, and the extra
includesonly runs for arrays the plugin has shifted or reordered.Tests
Added cases to
__tests__/base.jsfor push followed byunshift,shift,splice,reverseandsort, plus one where the nested draft is also modified. They check the nested value is the finalized (non-draft) object and that forward and inverse patches round-trip. They fail onmainin the array-plugin suite and pass with the fix.yarn test:src: 25 files, 4267 passedyarn build+vitest run --config vitest.config.build.ts: 3716 passed