Conversation
|
.... 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 |
|
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 Agreed that nobody writes If you'd rather not guard against this, no problem, feel free to close it. |
With
enableArrayMethods(),sliceandspliceon a draft resolve their index arguments withnormalizeSliceIndex, which only handles negative values and clamping. The native methods first convert the argument to an integer (fractions are truncated,NaNbecomes0), so non-integer indices behave differently from plain arrays: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, sosplice(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: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 thespliceno-op pre-check consistent forNaNstarts.Tests were added in
__tests__/base.jsfor bothslice(compared against the native result for several non-integer arguments) andsplice; they fail on the array-plugin configurations before the change.yarn test:srcpasses.