Repository navigation
fix(pullAt): fix incorrect element removal with negative indices - #1613
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
Fixes pullAt so multiple negative indices remove the intended elements by converting negative indices to their positive equivalents before splicing, preventing index drift caused by earlier splice() calls.
Changes:
- Normalize negative indices to positive indices prior to sorting/deduping and splicing.
- Add unit tests covering single, multiple, and mixed negative indices.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/array/pullAt.ts |
Normalizes negative indices before sorting and splicing to avoid incorrect removals. |
src/array/pullAt.spec.ts |
Adds coverage for negative-index scenarios (single, multiple, and mixed). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| const normalizedIndices = indicesToRemove.map(index => (index < 0 ? arr.length + index : index)); |
There was a problem hiding this comment.
normalizedIndices only adds arr.length for negative values, but doesn’t apply the same integer coercion / bounds handling as at(). This can cause pullAt to mutate the array for out-of-range negative indices (e.g. index < -arr.length becomes a negative normalized index and splice will remove from the end) even though at() returns undefined for those keys. Consider normalizing indices using the same rules as at() (truncate non-integers, add length for negatives) and then filtering out indices that are still out of [0, length) before splicing, so nonexistent indices don’t remove unrelated elements.
| const normalizedIndices = indicesToRemove.map(index => (index < 0 ? arr.length + index : index)); | |
| const length = arr.length; | |
| const normalizedIndices = indicesToRemove | |
| .map((index) => { | |
| let idx = Math.trunc(index); | |
| if (idx < 0) idx += length; | |
| return idx; | |
| }) | |
| .filter((idx) => idx >= 0 && idx < length); |
| it('should work with multiple negative indices', () => { | ||
| const array = [10, 20, 30, 40, 50]; | ||
| const result = pullAt(array, [-1, -2]); | ||
|
|
||
| expect(array).toEqual([10, 20, 30]); | ||
| expect(result).toEqual([50, 40]); | ||
| }); |
There was a problem hiding this comment.
The new negative-index tests cover in-range negatives, but there’s still no coverage for out-of-range negatives (e.g. index < -array.length). Given at() returns undefined for those, it would be good to add a case asserting pullAt also returns undefined and does not remove any element for such indices.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1613 +/- ##
=======================================
Coverage 99.85% 99.85%
=======================================
Files 499 499
Lines 4670 4671 +1
Branches 1357 1358 +1
=======================================
+ Hits 4663 4664 +1
Misses 7 7 🚀 New features to boost your workflow:
|
CI only lints changed .js/.ts files, so this .vue file was never linted when it was merged. Its curly-rule violations surface as full-lint failures on any PR whose stale base.sha makes eslint.config.mjs appear changed (e.g. #1613).
##### [v1.52.0](https://github.com/toss/es-toolkit/blob/HEAD/CHANGELOG.md#Version-v1520) Released on August 28th, 2026. - Added the `es-toolkit/iterator` entrypoint, a lazy iterator module with `chunk`, `count`, `dropWhile`, `head`, `iterate`, `partition`, `range`, `scan`, `takeWhile`, `uniqBy`, and `zip` (\[[#1815](toss/es-toolkit#1815)]), plus `cartesianProduct` (\[[#2031](toss/es-toolkit#2031)]). The same functions are also available from `es-toolkit/fp/iterator`. - Added `defer` and `deferAsync` to `es-toolkit/util`. (\[[#2032](toss/es-toolkit#2032)]) - Added `flowAsync` to `es-toolkit/fp`, a promise-aware left-to-right composition. (\[[#2033](toss/es-toolkit#2033)]) - Added the `EmptyObject`, `IsEqual`, `JSONValue`, `Primitive`, `SetOptional`, `SetRequired`, and `UnknownRecord` types. (\[[#2044](toss/es-toolkit#2044)]) - Fixed `merge` to infer the deep merged return type, and exported the `Merge` type. (\[[#1959](toss/es-toolkit#1959)]) - Fixed `omitBy` and `pickBy` to type numeric callback keys as strings, and exported the `ObjectKeys` type. (\[[#2037](toss/es-toolkit#2037)]) - Fixed `attempt` and `attemptAsync` to default their error type parameter to `unknown`. (\[[#2052](toss/es-toolkit#2052)]) The following bring `es-toolkit/compat` closer to Lodash. Behavior for the affected edge cases now matches Lodash, so results may differ if you relied on the previous output. - Fixed `compat/debounce` to skip the `maxWait` invocation when both `leading` and `trailing` are `false`. (\[[#1678](toss/es-toolkit#1678)]) - Fixed `compat/toString` to convert objects with a custom `valueOf`. (\[[#2034](toss/es-toolkit#2034)]) - Fixed `compat/mergeWith` to assign `null` values in array sources. (\[[#1929](toss/es-toolkit#1929)]) - Fixed `compat/has` and `compat/hasIn` to normalize the `-0` path key. (\[[#2035](toss/es-toolkit#2035)]) - Fixed `compat/differenceWith` and `compat/without` to normalize `-0` to `0`. (\[[#2049](toss/es-toolkit#2049)]) - Fixed `compat/findLastIndex` to convert `fromIndex` to an integer. (\[[#2047](toss/es-toolkit#2047)]) - Fixed `compat/xorWith` to match Lodash when no comparator is given. (\[[#2057](toss/es-toolkit#2057)]) - Covered the new iterator entrypoints in the dist and browser compatibility checks. (\[[#2030](toss/es-toolkit#2030)]) - Made the agent skills portable when installed on their own. (\[[#2054](toss/es-toolkit#2054)]) - Fixed documentation typos and the Deno install commands. (\[[#2062](toss/es-toolkit#2062)]) ##### [v1.51.0](https://github.com/toss/es-toolkit/blob/HEAD/CHANGELOG.md#Version-v1510) Released on August 17th, 2026. - Added the `es-toolkit/bigint` entrypoint: `bigint` counterparts of the math functions (`sum`, `sumBy`, `max`, `min`, `maxBy`, `minBy`, `clamp`, `inRange`, `median`, `medianBy`, `percentile`, `range`, `rangeRight`), kept separate so the `number` implementations stay unchanged. (\[[#1966](toss/es-toolkit#1966)]) - Added `dedent`, which removes common leading whitespace from template strings. (\[[#1679](toss/es-toolkit#1679)]) - Added `deepFreeze` for recursively freezing objects. (\[[#1680](toss/es-toolkit#1680)]) - Added `mapValuesAsync` and `mapKeysAsync`. (\[[#1544](toss/es-toolkit#1544)]) - Added `toPascalCaseKeys`, `toKebabCaseKeys`, and `toConstantCaseKeys`, completing the object key casing family. (\[[#1984](toss/es-toolkit#1984)]) - Exported the `ToCamelCaseKeys`, `ToPascalCaseKeys`, `ToSnakeCaseKeys`, `ToKebabCaseKeys`, and `ToConstantCaseKeys` types. (\[[#1651](toss/es-toolkit#1651)]) - Added a `preserveArrays` option to `flattenObject`. (\[[#974](toss/es-toolkit#974)]) - Fixed `retry` to pass the last error to the `delay` callback (\[[#1759](toss/es-toolkit#1759)]) and to throw the last attempt's error without applying a final delay (\[[#1901](toss/es-toolkit#1901)]). - Fixed `mergeWith` and `toMerged` to handle mixed array/object types consistently with `merge`. (\[[#2026](toss/es-toolkit#2026)]) - Fixed `pullAt` to remove the correct elements with negative indices. (\[[#1613](toss/es-toolkit#1613)]) - Fixed `at`, `pullAt`, `orderBy`, and `sortBy` to accept readonly arrays. (\[[#2016](toss/es-toolkit#2016)]) - Moved `limitAsync` from the array to the promise category. (\[[#2022](toss/es-toolkit#2022)]) - Included the NOTICE file in the published npm and JSR packages. (\[[#2020](toss/es-toolkit#2020)]) The following bring `es-toolkit/compat` closer to Lodash. Behavior for the affected edge cases now matches Lodash, so results may differ if you relied on the previous output. - Added `entries` and `entriesIn`. (\[[#1874](toss/es-toolkit#1874)]) - Fixed `compat/isMatch` and `compat/isMatchWith` to handle object source patterns against primitive and non-plain-object targets like Lodash. (\[[#1432](toss/es-toolkit#1432)]) - Fixed `compat/set` to block `constructor.prototype` paths, preventing prototype pollution. (\[[#1904](toss/es-toolkit#1904)]) - Fixed `compat/find` to respect array-like `length` (\[[#1958](toss/es-toolkit#1958)]) and to convert `fromIndex` to an integer (\[[#1961](toss/es-toolkit#1961)]), and `compat/indexOf` to convert `fromIndex` to an integer when searching for `NaN` (\[[#1951](toss/es-toolkit#1951)]). - Fixed `compat/orderBy` to resolve unresolved deep paths and nullish elements to `undefined` (\[[#2010](toss/es-toolkit#2010)]), to exclude the `length` property from array-like values (\[[#2002](toss/es-toolkit#2002)]), and to read the criterion property from primitive values (\[[#1903](toss/es-toolkit#1903)]). - Fixed `compat/toPath` to split unquoted dotted keys inside brackets. (\[[#1899](toss/es-toolkit#1899)]) - Fixed `compat/omit` to treat a key array as a single deep path. (\[[#1902](toss/es-toolkit#1902)]) - Fixed `compat/map` to make the iteratee optional with an identity default. (\[[#1928](toss/es-toolkit#1928)]) - Fixed `compat/size` to count combined Unicode symbols as single characters. (\[[#1900](toss/es-toolkit#1900)]) - Fixed `compat/intersectionBy` to support number, object, and nullish iteratee shorthands (\[[#1993](toss/es-toolkit#1993)]), and `compat/intersectionWith` to apply the comparator when deduplicating the first array (\[[#1953](toss/es-toolkit#1953)]). - Fixed `compat/difference`, `compat/differenceBy`, `compat/uniqBy`, and `compat/unionBy` to normalize `-0` to `0`. (\[[#2007](toss/es-toolkit#2007)]) - Fixed `compat/maxBy` and `compat/minBy` to skip nullish iteratee values (\[[#1927](toss/es-toolkit#1927)]), and `compat/meanBy` to skip `undefined` values when averaging (\[[#1978](toss/es-toolkit#1978)]). - Fixed `compat/some`, `compat/template` (\[[#2009](toss/es-toolkit#2009)]), and `compat/fill` (\[[#1952](toss/es-toolkit#1952)]) to gate guard handling with `isIterateeCall`. - Fixed `compat/iteratee` to treat a boolean as a property shorthand. (\[[#1941](toss/es-toolkit#1941)]) - Fixed `compat/reverse` to support array-like objects and `arguments`. (\[[#2008](toss/es-toolkit#2008)]) - Fixed `compat/toArray` to convert non-array-like iterables. (\[[#1999](toss/es-toolkit#1999)]) - Fixed `compat/toString` to render nested nullish values like Lodash. (\[[#1992](toss/es-toolkit#1992)]) - Fixed `compat/zipObject` to treat `null` keys and values as empty arrays. (\[[#2005](toss/es-toolkit#2005)]) - Fixed `compat/sampleSize` to clamp the default size to the collection length. (\[[#2003](toss/es-toolkit#2003)]) - Fixed `compat/takeWhile`, `compat/takeRightWhile`, and `compat/uniqBy` to support string inputs. (\[[#2004](toss/es-toolkit#2004)]) - Fixed `compat/chunk` to return an empty array for an empty collection with an `Infinity` size. (\[[#1995](toss/es-toolkit#1995)]) - Fixed `compat/lastIndexOf` to read holes in sparse arrays as `undefined`. (\[[#1977](toss/es-toolkit#1977)]) - Fixed `compat/last` to return `undefined` for empty array-like inputs. (\[[#1954](toss/es-toolkit#1954)]) - Fixed `compat/pullAll` to return the array as-is when `values` is nullish. (\[[#1962](toss/es-toolkit#1962)]) - Added a browser compatibility test suite. (\[[#2012](toss/es-toolkit#2012)]) - Improved documentation, JSDoc accuracy, tests, and internal refactoring across `compat` and the docs site. (\[[#1622](toss/es-toolkit#1622)], \[[#1789](toss/es-toolkit#1789)], \[[#1824](toss/es-toolkit#1824)], \[[#1844](toss/es-toolkit#1844)], \[[#1849](toss/es-toolkit#1849)], \[[#1854](toss/es-toolkit#1854)], \[[#1855](toss/es-toolkit#1855)], \[[#1859](toss/es-toolkit#1859)], \[[#1860](toss/es-toolkit#1860)], \[[#1868](toss/es-toolkit#1868)], \[[#1871](toss/es-toolkit#1871)], \[[#1887](toss/es-toolkit#1887)], \[[#1898](toss/es-toolkit#1898)], \[[#1933](toss/es-toolkit#1933)], \[[#1939](toss/es-toolkit#1939)], \[[#1947](toss/es-toolkit#1947)], \[[#1950](toss/es-toolkit#1950)], \[[#1955](toss/es-toolkit#1955)], \[[#1956](toss/es-toolkit#1956)], \[[#1957](toss/es-toolkit#1957)], \[[#1960](toss/es-toolkit#1960)], \[[#1963](toss/es-toolkit#1963)], \[[#1964](toss/es-toolkit#1964)], \[[#1968](toss/es-toolkit#1968)], \[[#1969](toss/es-toolkit#1969)], \[[#1970](toss/es-toolkit#1970)], \[[#1972](toss/es-toolkit#1972)], \[[#1979](toss/es-toolkit#1979)], \[[#1985](toss/es-toolkit#1985)], \[[#2000](toss/es-toolkit#2000)], \[[#2001](toss/es-toolkit#2001)], \[[#2021](toss/es-toolkit#2021)], \[[#2025](toss/es-toolkit#2025)], \[[#2027](toss/es-toolkit#2027)]) We sincerely thank [@Antoliny0919](https://github.com/Antoliny0919), [@D-Sketon](https://github.com/D-Sketon), [@kojesung](https://github.com/kojesung), [@chuenchen309](https://github.com/chuenchen309), [@mariazuheros](https://github.com/mariazuheros), [@DongEun02](https://github.com/DongEun02), [@raon0211](https://github.com/raon0211), [@sukvvon](https://github.com/sukvvon), [@racgoo](https://github.com/racgoo), [@dayongkr](https://github.com/dayongkr), [@Kropiunig](https://github.com/Kropiunig), [@ssi02014](https://github.com/ssi02014), [@sarathfrancis90](https://github.com/sarathfrancis90), [@mym0404](https://github.com/mym0404), [@mlnwns](https://github.com/mlnwns), [@kdelay](https://github.com/kdelay), [@gwagjiug](https://github.com/gwagjiug), [@dohwi](https://github.com/dohwi), [@blro0319](https://github.com/blro0319), [@benedictleejh](https://github.com/benedictleejh), [@amir-rezaei](https://github.com/amir-rezaei), [@ianzone](https://github.com/ianzone), [@sen2y](https://github.com/sen2y), [@rossyman](https://github.com/rossyman), [@eunwoo-levi](https://github.com/eunwoo-levi), [@ramong26](https://github.com/ramong26), [@Gdm0714](https://github.com/Gdm0714), and everyone else who contributed. We appreciate your great efforts! Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Problem
When multiple negative indices are passed to
pullAt, earliersplicecalls shift the array length, causing subsequent negative indices to reference wrong elements.Solution
Normalize negative indices to positive indices before splicing, so that array length changes don't affect which elements are removed.
Test