Skip to content

fix: preserve Map keys during finalization and patch replay - #1312

Open
snooze26h wants to merge 1 commit into
immerjs:mainfrom
snooze26h:fix/preserve-map-keys
Open

snooze26h wants to merge 1 commit into
immerjs:mainfrom
snooze26h:fix/preserve-map-keys

Conversation

@snooze26h

Copy link
Copy Markdown

This PR proposes a fix for a few Map-key cases in draft finalization and patch replay. They reproduce in Immer 11.1.21 and on main at 8848a5b:

  • Updating an entry keyed by undefined leaves a revoked draft in the result.
  • Replaying a nested patch through a null, boolean, object, or symbol key fails because the path walker converts the key to a string.
  • Adding, replacing, or removing an entry with an array key splits that key into multiple patch-path segments.

For example:

import {enableMapSet, produce} from "immer"

enableMapSet()
const base = new Map([[undefined, {count: 0}]])
const next = produce(base, draft => {
  draft.get(undefined).count++
})
next.get(undefined).count // Throws: proxy has been revoked

The patch identifies child drafts by their parent relationship, preserves Map keys during path traversal, and appends each assigned key as a single path segment. The existing object/array prototype-pollution guards are retained, with regression coverage for reserved attributes below Map entries.

Related background: #952 and #1025 addressed numeric Map keys.

Validation

Node.js 24.20.0, macOS arm64, Yarn 1.22.22 via Corepack:

  • 64 new tests covering produced values, patch replay and undo, key identity, structural sharing, removed/replaced children, and both auto-freeze modes. 48 regression cases were demonstrated failing before the fix.
  • yarn test: 3,900 source tests and 3,349 production-build tests passed; Flow reported 0 errors. Each suite retains 8 pre-existing skips.
  • yarn coverage and the existing yarn test:perf scripts completed successfully.
  • Prettier checks for the changed files and git diff --check passed.
  • A separate public-API reproduction fails on the published package and passes against the patched production build.

I'd appreciate any feedback on the approach or test coverage, and I'm happy to make adjustments. Thanks for taking a look.

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