Skip to content

fix: preserve removed drafts with optimized array methods - #1349

Open
jipeng6036-del wants to merge 1 commit into
immerjs:mainfrom
jipeng6036-del:fix/array-methods-preserve-removed-drafts
Open

jipeng6036-del wants to merge 1 commit into
immerjs:mainfrom
jipeng6036-del:fix/array-methods-preserve-removed-drafts

Conversation

@jipeng6036-del

Copy link
Copy Markdown

With enableArrayMethods(), a recipe that moves an object between lists and then updates it can mutate the original state, or throw when the base is frozen:

produce({todo: [{done: false}], done: []}, draft => {
  const item = draft.todo.pop()
  draft.done.push(item)
  item.done = true
})

Read only the removed element through the existing draft proxy before optimized pop / shift. Use the normal proxy path for splice, preserving removed drafts, native argument coercion and sparse-array behavior. Newly inserted external objects keep their existing undrafted behavior.

splice consequently has the standard proxy cost; pop / shift retain their internal-copy optimization. The documentation makes that tradeoff explicit.

Validation:

  • 32 regression cases: original source 10 failures / 22 controls pass; candidate all pass, including frozen inputs, patches/undo, prior draft identity, external values, getters and no-op sharing.
  • Windows and Linux: yarn test (4,244 source and 3,693 production tests pass; 8 skips in each), Flow, coverage and all four existing performance scripts passed. The unchanged fork Test workflow also passed on this commit.
  • Fresh consumers installed actual local npm pack artifacts: 102 public CJS/ESM checks pass across Windows/Linux; baseline defects reproduce. These are local artifacts, not a registry release.

Reproduction, exact artifacts, independent checks and size/performance limitations. The size tool needed a size-only OpenSSL compatibility option; the original failed measurements are retained separately.

Prepared and validated with Codex.

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