Automattic / Automattic/expect.js

eql does not work if an array item is undefined in IE8

Open
#140 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
JavaScript
Stars
2.1k
Forks
207
PR merge metrics
No merged PRs in 30d

Description

The following test fails

``` js
describe('eql', function () {
it('should not remove undefined', function () {
var arr = []
arr.push('1')
arr.push('2')
arr.push(undefined)
expect(arr).to.eql(['1', '2', undefined])
})
})
```

This is because the `keys` function used [here](https://github.com/Automattic/expect.js/blob/master/index.js#L941) will not iterate over an undefined array item in IE8 **unless the value was pushed to the array**.

If on line 941 the keys function is called on a duplicate of the array, this could be resolved.

``` js
try{
var ka, kb, key, i;
if (isArray(a) && isArray(b)) {
ka = keys(cloneArray(a));
kb = keys(cloneArray(b));
} else {
ka = keys(a);
kb = keys(b);
}
} catch (e) {//happens when one is a string literal and the other isn't
return false;
}

function cloneArray (arr) {
var clone = [];
for (var i = 0; i < arr.length; i++) {
clone.push(arr[i]);
}
return clone;
}

function isArray (obj) {
return Object.prototype.toString.call(obj) === '[object Array]';
}
```

Contributor guide

No contributing guide indexed for this repository

Research direction

Start at index.js around line 941 and reproduce the eql test shown in the issue in IE8, focusing on how keys handles arrays containing a pushed undefined value. Done means the test passes and eql preserves that undefined array item without changing the existing non-array behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
testing
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
50/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.