js-polyfills
js-polyfills copied to clipboard
Callback bug in findLastIndex
Hi Siddhi,
The findLastIndex function has a small bug where it invokes callback with undefined as the first argument before moving on to the other items. Not always a problem, but if the callback is expecting an object then it can cause exceptions to be thrown - e.g. for callback element => element.foo = 'bar'.
The fix is changing this:
for (let i = this.length; i >= 0; i--) {
to this:
for (let i = this.length - 1; i >= 0; i--) {
We can also add the following test to make sure this is fixed and won't happen again:
test("invokes callback in the expected way", () => {
const arr = [123, 456, 789];
const callback = jest.fn(() => false);
arr.myFindLastIndex(callback);
expect(callback).toHaveBeenCalledTimes(3);
expect(callback).toHaveBeenNthCalledWith(1, 789, 2, arr);
expect(callback).toHaveBeenNthCalledWith(2, 456, 1, arr);
expect(callback).toHaveBeenNthCalledWith(3, 123, 0, arr);
});
And also maybe this to make sure we're not calling callback more often than needed:
test("does not invoke callback after element found", () => {
const arr = [123, 456, 789];
const callback = jest.fn((element) => element === 456);
arr.myFindLastIndex(callback);
expect(callback).toHaveBeenCalledTimes(2);
expect(callback).toHaveBeenNthCalledWith(1, 789, 2, arr);
expect(callback).toHaveBeenNthCalledWith(2, 456, 1, arr);
});
I was going to open a PR for you, but I don't have permission, hopefully the above is simple for you to use anyway.
Thanks!