Skip to content

fix: truncate fractional slice and splice indices in the array methods plugin - #1313

Open
kevin9327 wants to merge 1 commit into
immerjs:mainfrom
kevin9327:fix-array-methods-fractional-index
Open

kevin9327 wants to merge 1 commit into
immerjs:mainfrom
kevin9327:fix-array-methods-fractional-index

Conversation

@kevin9327

Copy link
Copy Markdown

With enableArrayMethods(), slice and splice on a draft resolve their index arguments with normalizeSliceIndex, which only handles negative values and clamping. The native methods first convert the argument to an integer (fractions are truncated, NaN becomes 0), so non-integer indices behave differently from plain arrays:

enableArrayMethods()
const base = [{v: 0}, {v: 1}, {v: 2}, {v: 3}, {v: 4}]

produce(base, draft => {
  draft.slice(0, 2.5) // 3 items, native: 2
  draft.slice(1.5)    // [undefined, undefined, undefined, undefined], native: items 1..4
  draft.slice(NaN)    // [], native: all 5 items
})

Something like draft.items.slice(0, draft.items.length / 2) on an odd-length array hits this.

For splice, the inserted values are registered under the index computed by the same helper, so splice(1.5, 0, value) marks key "1.5" instead of "1". If the inserted value references another draft, that reference is never finalized and the result contains a revoked proxy:

const base = {items: [{id: 1}, {id: 2}], other: {x: 1}}
const result = produce(base, draft => {
  draft.items.splice(1.5, 0, {ref: draft.other})
  draft.other.x = 2
})
JSON.stringify(result) // TypeError: Cannot perform 'get' on a proxy that has been revoked

The fix converts the index the way the native methods do (Math.trunc(Number(index)) || 0) before the existing negative/clamping logic. This also keeps the splice no-op pre-check consistent for NaN starts.

Tests were added in __tests__/base.js for both slice (compared against the native result for several non-integer arguments) and splice; they fail on the array-plugin configurations before the change. yarn test:src passes.

@markerikson

markerikson commented Oct 5, 2026 •

Copy link
Copy Markdown
Collaborator

.... granted this is a 1-line fix, but...

who in their right mind would actually pass a fractional index into array methods like this?

I've been writing JS for over a decade and had never even considered that someone might do that, and I've definitely never seen it in practice.

I would argue this isn't a thing we need to guard against. what prompted you to file this one?

(I'll agree that the items.length / 2 scenario is at least theoretically possible.)

@kevin9327

Copy link
Copy Markdown
Author

Fair question. I ran into it while comparing the array-methods plugin against plain arrays: the plugin is meant to be a drop-in for the native methods, and native slice/splice truncate the index, so the same reducer gives a different result depending on whether the plugin is enabled.

Agreed that nobody writes slice(0, 2.5) on purpose. The realistic case is a computed index like items.length / 2 or a value from a slider/percentage. The part that worried me more than the off-by-one is the splice case: with a fractional start, the inserted draft ends up as a revoked proxy in the result, so it fails later with a "revoked proxy" error far away from the cause.

If you'd rather not guard against this, no problem, feel free to close it.

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.

2 participants