Skip to content

fix: finalize drafts nested in inserted values after the array methods plugin moves them - #1371

Open
waw666waw666 wants to merge 1 commit into
immerjs:mainfrom
waw666waw666:fix-array-methods-nested-drafts
Open

waw666waw666 wants to merge 1 commit into
immerjs:mainfrom
waw666waw666:fix-array-methods-nested-drafts

Conversation

@waw666waw666

Copy link
Copy Markdown

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:

import {produce, enableArrayMethods} from "immer"
enableArrayMethods()

const next = produce([{id: 1}, {id: 2}], draft => {
  draft.push({ref: draft[1]})
  draft.unshift({id: 0}) // same with shift, splice, sort or reverse
})

JSON.stringify(next) // TypeError: Cannot perform 'get' on a proxy that has been revoked

Without the plugin the same recipe works, because the native methods write each moved element through the set trap and register a new callback for its new index.

Cause

handleCrossReference registers 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 in assigned_). The plugin's shift, unshift, splice, sort and reverse move elements on copy_ directly and set allIndicesReassigned_ 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 includes only runs for arrays the plugin has shifted or reordered.

Tests

Added cases to __tests__/base.js for push followed by unshift, shift, splice, reverse and sort, 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 on main in the array-plugin suite and pass with the fix.

  • yarn test:src: 25 files, 4267 passed
  • yarn build + vitest run --config vitest.config.build.ts: 3716 passed
  • prettier applied

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant